[Geany-devel] [geanygdb plugin] Stack overflow when activating it.

[Enrico Tröger]

On Wed, 02 Sep 2009 21:16:03 +0400, Eugene wrote:

>Enrico Tro"ger wrote:
>> On Wed, 02 Sep 2009 20:48:17 +0400, Eugene wrote:
>>
>>   
>>> Hi all.
>>>
>>> The trouble is caused by unsafe usage of `strncpy' and long strings
>>> in Russian localization. I found that the stack overflow is caused
>>> by the following code (geany-plugins/geanygdb/src/gdb-ui-main.c :
>>> 366)
>>>
>>> -------------------- 8< --------------------
>>>
>>> if (text && disable_mnemonics)
>>>    {
>>>        gchar *p;
>>>        gchar buf[32];
>>>        strncpy(buf, text, sizeof(buf));
>>>        for (p = buf; *p; p++)
>>>        {
>>>            if (*p == '_')
>>>            {
>>>                memmove(p, p + 1, strlen(p));
>>>            }
>>>        }
>>>        text = buf;
>>>    }
>>>
>>> -------------------- 8< --------------------
>>>
>>> Obviously, 32 chars are enough for English localisation, but not
>>> for Russian one (which I am using). Quick fix:
>>>
>>> ...
>>> gchar buf[64];
>>> strncpy(buf, text, sizeof(buf)-1);
>>> buf[sizeof(buf)-1] = '\0';
>>>     
>>
>> This is not really better. Increasing the buffer size only works as
>> long as someone appears with another language which needs even more
>> characters. Ok, at some point it gets unlikely if the buffer size is
>> big enough but it's still ugly.
>>
>> I suggest a more easy and secure approach (even being a little bit
>> slower):
>>
>> gchar *buf = g_strdup(text);
>>
>> This is very unlikely to fail except there is no more free memory on
>> the heap but well, in this case many many more things in Geany would
>> go wrong...:)
>>   
>Agree, g_strdup will be even better.
>>
>> Btw,
>> buf[sizeof(buf)-1]
>>
>> is really wrong. Because sizeof(buf) is always 32 as it is a fixed
>> sized char array. But you want to put the \0 at the end of the actual
>> content not at the end of the buffer.
>>   
>Putting \0 at the end of the buffer would be enough. If content is 
>smaller than the buffer, strncpy itself will append \0.

Hmm, right.
Still, IMO it's weird to set \o at sizeof(buf)-1.


Regards,
Enrico

-- 
Get my GPG key from http://www.uvena.de/pub.asc
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.geany.org/pipermail/devel/attachments/20090902/88f4b181/attachment.pgp>