[Bf-committers] Blender 2.43 RC1

[Giuseppe Ghibò]

Ton Roosendaal ha scritto:

> Hi,
> 
>> Because it crashes immediately with buffer overflows
>> (though the code apparently seems correct). See my older mail.
> 
> When does this crash?
> It is not the nicest code in the world though...  will commit something 
> else that works.

the crash happens immediately at the beginning after opening the main window, 
simply launching blender.

> 
> Note btw that using snprintf() will create a bug (adds trailing zero to 
> string). That was excactly the reason why I coded a 1 byte 'buffer 
> overflow' into a safe area.

what do you mean as "safe area"?

AFAIK snprintf() will pad trailing zero, only if it would print
fewer char than requested, but AFAIK the trailing zeroes or
the total amount of character written shouldn't
exceed the amount of size specified.

Indeed mine solution as you stated is not totally right, nor the old one; we have:

typedef struct FileGlobal {
         char subvstr[4];
         ...
}

in ./source/blender/makesdna/DNA_fileglobal_types.h; indeed we have
BLENDER_SUBVERSION which is 3, and fg.subvstr which is a buffer of
4 elements. So with original code we have:

fb.subvstr[0] = ' '; (space, 32)
fb.subvstr[1] = ' '; (space, 32)
fb.subvstr[2] = ' '; (space, 32)
fb.subvstr[3] = '3'; (51)
fb.subvstr[4] = '\0'; (0) /* this is buffer overflow as fb.subvstr[4]
                              doesn't exists and shouldn't be written */

while with mine:

fb.subvstr[0] = ' '; (space, 32)
fb.subvstr[1] = ' '; (space, 32)
fb.subvstr[2] = ' '; (space, 32)
fb.subvstr[3] = '\0'; (0)
fb.subvstr[4] /* don't exists an it's not written (so would contain junk or
               nearest element structure) */

in other words don't have overflow, but the fb.subvstr doesn't contain
the right value. The right solution IMHO is indeed this:

snprintf(fg.subvstr, sizeof(fg.subvstr), "%3d", BLENDER_SUBVERSION);

or

sprintf(fg.subvstr, "%3d", BLENDER_SUBVERSION);

which would lead to have:

fb.subvstr[0] = ' '; (space, 32)
fb.subvstr[1] = ' '; (space, 32)
fb.subvstr[2] = '3'; (space, 32)
fb.subvstr[3] = '\0'; (0)

in both cases, or alternatively change the subvstr size in struct FileGlobal to 
5, but thus affecting also the ABI.

WDYT?

Bye
Giuseppe.

> 
> -Ton-
> 
>>
>> Bye
>> Giuseppe.
>>
>>>
>>> -Ton-
>>>
>>> On 8 Jan, 2007, at 9:36, Giuseppe Ghibò wrote:
>>>
>>>> OK. I'll test. BTW, would be possible to commit also the one at:
>>>>
>>>> source/blender/blenloader/intern/writefile.c
>>>>
>>>> line 1853:
>>>>
>>>> sprintf(fg.subvstr, "%4d", BLENDER_SUBVERSION);
>>>>
>>>> with:
>>>>
>>>> snprintf(fg.subvstr, sizeof(fg.subvstr), "%4d", BLENDER_SUBVERSION);
>>>>