Re: [code] PATCH: fix a heap buffer overflow

From: Mitchell <m.att.foicica.com>
Date: Wed, 17 Feb 2016 09:29:14 -0500 (EST)

Hi Markus,

On Wed, 17 Feb 2016, Markus F.X.J. Oberhumer wrote:

> Hi Mitchell,
>
> On 2016-02-17 02:52, Mitchell wrote:
>> Hi Markus,
>>
>> On Mon, 15 Feb 2016, Markus F.X.J. Oberhumer wrote:
>>
>>> Please see attached patch and log.
>>>
>>> BTW, I can really recommend "-fsanitize=address" to all C/C++ users.
>>> For a for a relatively modest performance slowdown you do get a lot
>>> of useful runtime checks.
>>
>> I'm not sure I agree with the output of that tool. Scintilla initializes
>> SCNotifications like this:
>>
>> SCNotification scn = {};
>>
>> Now I am no expert on this, but according to the C standard, that type of
>> initializer zeros out all structure members. Therefore accessing `scn->text`
>> and passing its result to `lua_pushstring` will always result in a zero read
>> length (since `lua_pushstring` works with C strings).
>>
>> Could you shed some more light on this? I'm not sure how to interpret the
>> output of your tool :(
>
> I think the problem is that the string that "n->text" points to is
> not zero terminated in all cases - that's why we have to respect "n->length"
> and must use "lua_pushlstring(L, n->text, n->length);"

Thank you for your clear explanation. That makes sense. After looking at
Scintilla's documentation, it looks like if `n->text` and `n->length` are
given, the string may not be NUL terminated. If only `n->text` is given,
the string is NUL terminated. I'll commit a fix that handles these cases.

Cheers,
Mitchell

-- 
You are subscribed to code.att.foicica.com.
To change subscription settings, send an e-mail to code+help.att.foicica.com.
To unsubscribe, send an e-mail to code+unsubscribe.att.foicica.com.
Received on Wed 17 Feb 2016 - 09:29:14 EST

This archive was generated by hypermail 2.2.0 : Thu 18 Feb 2016 - 06:52:24 EST