[Zlib-devel] potential overflows by sprintf/vsprintf in gzio.c

[Glenn Randers-Pehrson]

At 10:16 PM 4/5/03 -0800, Mark Adler wrote:
>On Saturday, April 5, 2003, at 04:33  AM, Glenn Randers-Pehrson wrote:
>> Lines 575 and 616 do check for len >= sizeof(buf) but that appears to
>> be unnecessary because len was returned by vsnprintf or snprintf,
>> respectively, which should have already checked the length and
>> returned a string that is within the sizeof(buf).
>
>Those functions don't return what you think they return.  They return 
>the number of characters that would be written given enough room.  So 
>if the returned value is greater than or equal to the provided size, 
>then the string was terminated early (to fit in the allowed size) and 
>some of the output was discarded.  So the check is correct.

OK.  But it's detecting a failure to write the entire string, not an
overflow.

>> On the other hand, lines 558 and 597 do not check for overflow, but
>> here len was returned by vsprintf or sprintf, respectively, which
>> don't check the length.
>
>At this point, the damage is already done.  If the available memory was 
>exceeded, then there's no reason to expect a strlen() to work anyway, 
>since something else may have properly written over the end of the 
>string in the meantime.  Having strlen() go off looking for a zero 
>somewhere in memory may just make matters worse.

How about allocating a guard byte and checking to make sure it is
still zero after the write?

    char buf[Z_PRINTF_BUFSIZE+1];
    ...
    buf[Z_PRINTF_BUFSIZE]='\0';
    ...
    vsprintf(...);
    if (buf[Z_PRINTF_BUFSIZE])
      error--overflow has occurred

Glenn