apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@attglobal.net>
Subject Re: cvs commit: apr/strings apr_strings.c
Date Wed, 30 Jun 2004 10:00:29 GMT
David Reid wrote:
>>wrowe@apache.org wrote:
>>
>>>wrowe       2004/06/28 11:09:16
>>>
>>>  Modified:    strings  Tag: APR_0_9_BRANCH apr_strings.c
>>>  Log:
>>>    Avoid any edge case or clib bug that might result in a string
>>>    overflow of the fixed 5-byte buffer for our size function.
>>>    Returns the '****' string when the buffer would overflow.
>>>    Backport of rev 1.47
>>>  
>>>  Reviewed by: trawick
>>
>>reviewed but not approved ;)
>>
>>it still has the same bug (apr_snprintf() doesn't return < 0 either)
> 
> 
> Care to supply a fixed version?

IMHO the original version is sufficient for now.  In fact, the new version 
isn't going to misbehave; it just introduces dead code.

One way of implementing the spirit of what Bill Rowe was trying to do would 
require looking at the generated buffer instead of the return code from 
sprintf/apr_snprintf.

For example, change

sprintf(buf, "%3d ", (int) size);

to

apr_snprintf(buf, 5, "%3d ", (int) size);
if (buf[3] != ' ') { /* catch overflow */
     return strcpy(buf, "****");
}

(For the other changes we'd have to look for the final 'c' in a certain 
position to detect overflow.)

Alternatively, apr_snprintf into a larger local buffer to be able to detect 
overflow of a 5-byte buffer by looking at the apr_snprintf() return code, and 
if okay copy the output to the caller's buffer before returning.

Note that libc has to have broken sprintf() or somebody has to introduce a new 
bug into the apr_strfsize() function in order to have such an overflow anyway. 
  Due to the API, we can't catch the problem where the caller passes a buffer 
which is not large enough.


Mime
View raw message