On Wed, 2 Feb 2005, Daniel Stenberg wrote:

> On Wed, 2 Feb 2005, David Byron wrote:
>
> > How does this work for you all?
>
> > do {
> > size_t buffersize = data->set.buffer_size?
> > - data->set.buffer_size:BUFSIZE -1;
> > + data->set.buffer_size:BUFSIZE;
>
> Sorry, but I don't believe it is a good fix. This is exactly
> the same change as increasing the buffer size define with
> one.
>
> The reason for the -1 in there is found further down:
>
> /* NULL terminate, allowing string ops to be used */
> if (0 < nread || is_empty_data)
> k->buf[nread] = 0;
>
> ... so with your patch, there's a single-byte buffer overflow.

I don't think there's a buffer overflow by doing this. The actual
buffer is an array of [BUFSIZE + 1], so there's still room for the NULL
terminator. Also, in http.c, the call to Curl_read passes BUFSIZE when
nread is 0, so I think BUFSIZE is OK.

Not that this is a bulletproof test, but I created another document
that's one byte shorter, and another that's one byte longer and the
patched curl downloads them both just fine.

-DB
Received on 2005-02-03