httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: Possible error in apr_rgetline_core
Date Tue, 21 Sep 2004 16:03:27 GMT
On Tue, Sep 21, 2004 at 10:00:48AM -0500, Rici Lake wrote:
> The first time through this loop, bytes_handled will be 0. If the 
> buffer was provided, rather than being allocated by ap_rgetline_core, 
> and the first read exceeded the maximum length (n), then line 256 will 
> set the byte *before* the buffer to 0.

you know, I had even traced this through and checked that bytes_handled
must be > 0 if *s is non-NULL, conveniently forgetting about the
user-supplied *s case :(

> I believe that the change introduced at revision 1.152 is incorrect; 
> the test at line 255 ensures that the buffer will not be overrun, so 
> the change from bytes_handled to bytes_handled-1 in line 256 was 
> unnecessary.

I don't agree with this, though... trace through the old code with n =
8192, for two reads of len=8191, and len=5:

pass 1: bytes_handled = 0, *s = NULL

apr_bucket_read() => len = 8191
if (n < bytes_handled + len) => false
current_alloc = 8191
*s = apr_palloc(, 8191)
bytes_handled += 8191

pass 2: bytes_handled = 8191, *s = 8191-byte region

apr_bucket_read() => len = 5
if (n < bytes_handled + len) : 8192 < 8191 + 5 => true
*read = 8191
if (*s) => true
if (bytes_handled < n) : 8191 < 8192 => true
(*s)[8191] = '\0' 
     => off-by-one


Mime
View raw message