On 04/24/2009 10:10 PM, C. Michael Pilato wrote:
>
> /* tack on null terminator to remaining string */
> *(vd.vbuff.curpos) = '\0';
>
> Uh-oh. Buffer overflow!
>
> Our CollabNet engineer is proposing a simple fix: defining 'buf' inside
> apr_brigade_vprintf() like so:
>
> char buf[APR_BUCKET_BUFF_SIZE + 1]
>
> (Note the "+ 1" to make room for that pesky NULL byte.)
>
> But I'm wondering if an equally correct fix would be to simply not tack the
> '\0' onto the buffer at all. Doesn't apr_brigade_write() accept both the
> buffer and the number of bytes to write? Does it really need a
> null-terminated string, especially considering that its input could be
> arbitrary binary data? Other calls to it pass things like "str" and
> "strlen(str)", which would ignore the NULL byte in "str".
>
> [1]
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&viewType=browseAll&dsMessageId=1745697
>
Fixed in r768417 (http://svn.apache.org/viewvc?view=rev&revision=768417).
Regards
Rüdiger
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1897608
Received on 2009-04-25 03:01:57 CEST