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

[Mark Adler]

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.

> 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.

It occurs to me that a safe thing to do would be to look for the end of 
the string up to the maximum length and then give up.  If it gives up, 
then it can return zero to at least provide an indication of the 
disaster already in the making.  I think I'll add that, though it is of 
limited usefulness.

> I'm still a little unhappy though because the checks occur after
> the buf has potentially been overflowed.  I don't see an easy way
> around that problem, though.

There is no way around it, since sprintf() and vsprintf() are 
inherently insecure.  The other thing that zlib 1.2.0.1 does is warn 
whoever is compiling zlib that it has been configured to use insecure 
functions if snprintf() or vsnprintf() is not available.

mark