You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <rh...@sharpsvn.net> on 2009/02/19 01:18:50 UTC
RE: svn commit: r35968 - trunk/subversion/libsvn_subr
> -----Original Message-----
> From: Bert Huijben [mailto:rhuijben@sharpsvn.net]
> Sent: Thursday, February 19, 2009 2:13 AM
> To: svn@subversion.tigris.org
> Subject: svn commit: r35968 - trunk/subversion/libsvn_subr
>
> Author: rhuijben
> Date: Wed Feb 18 17:13:10 2009
> New Revision: 35968
>
> Log:
> * subversion/libsvn_subr/svn_string.c
> (svn_stringbuf_create_ensure): Make sure the '\0' always fits in the
> buffer
> to avoid a possible (and proven) buffer overflow.
>
> This bug caused an assertion on the windows debug heap in the neon log
> tests
> for 1.6.0rc1.
01:49 <@Bert> gstein: svn_stringbuf_create_ensure is called with blocksize 0
when uri_escape is called with a "" path..
this writes data to a 0 byte block
01:50 <@gstein> hmm
01:51 <@gstein> eeps. yeah, it does!
And this can be caused by joining an empty path to an uri, which in turn
caused my neon failure.
Bert
>
> Modified:
> trunk/subversion/libsvn_subr/svn_string.c
>
> Modified: trunk/subversion/libsvn_subr/svn_string.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/svn_strin
> g.c?pathrev=35968&r1=35967&r2=35968
> =======================================================================
> =======
> --- trunk/subversion/libsvn_subr/svn_string.c Wed Feb 18 14:47:29 2009
> (r35967)
> +++ trunk/subversion/libsvn_subr/svn_string.c Wed Feb 18 17:13:10 2009
> (r35968)
> @@ -236,7 +236,7 @@ create_stringbuf(char *data, apr_size_t
> svn_stringbuf_t *
> svn_stringbuf_create_ensure(apr_size_t blocksize, apr_pool_t *pool)
> {
> - char *data = apr_palloc(pool, blocksize);
> + char *data = apr_palloc(pool, ++blocksize); /* + space for '\0' */
>
> data[0] = '\0';
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=1189179
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1189210
RE: svn commit: r35968 - trunk/subversion/libsvn_subr
Posted by Chris Foote <ca...@yahoo.com>.
Hi Bert,
According to the API contract for svn_stringbuf_create_ensure the
'minimum_size should include space for the terminating NULL character'. So I think
that it is the caller(s) of this function that are not correct. In most places the
minimum_size parameter is passed as size+1 to allocate space for the NULL.
This also causes a test failure:
FAIL: lt-string-test 10: block initialization and growth
Regards,
Chris
> > -----Original Message-----
> > From: Bert Huijben [mailto:rhuijben@sharpsvn.net]
> > Sent: Thursday, February 19, 2009 2:13 AM
> > To: svn@subversion.tigris.org
> > Subject: svn commit: r35968 - trunk/subversion/libsvn_subr
> >
> > Author: rhuijben
> > Date: Wed Feb 18 17:13:10 2009
> > New Revision: 35968
> >
> > Log:
> > * subversion/libsvn_subr/svn_string.c
> > (svn_stringbuf_create_ensure): Make sure the '\0' always fits in the
> > buffer
> > to avoid a possible (and proven) buffer overflow.
> >
> > This bug caused an assertion on the windows debug heap in the neon log
> > tests
> > for 1.6.0rc1.
>
> 01:49 <@Bert> gstein: svn_stringbuf_create_ensure is called with blocksize 0
> when uri_escape is called with a "" path..
> this writes data to a 0 byte block
> 01:50 <@gstein> hmm
> 01:51 <@gstein> eeps. yeah, it does!
>
> And this can be caused by joining an empty path to an uri, which in turn
> caused my neon failure.
>
> Bert
>
> >
> > Modified:
> > trunk/subversion/libsvn_subr/svn_string.c
> >
> > Modified: trunk/subversion/libsvn_subr/svn_string.c
> > URL:
> > http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/svn_strin
> > g.c?pathrev=35968&r1=35967&r2=35968
> > =======================================================================
> > =======
> > --- trunk/subversion/libsvn_subr/svn_string.c Wed Feb 18 14:47:29 2009
> > (r35967)
> > +++ trunk/subversion/libsvn_subr/svn_string.c Wed Feb 18 17:13:10 2009
> > (r35968)
> > @@ -236,7 +236,7 @@ create_stringbuf(char *data, apr_size_t
> > svn_stringbuf_t *
> > svn_stringbuf_create_ensure(apr_size_t blocksize, apr_pool_t *pool)
> > {
> > - char *data = apr_palloc(pool, blocksize);
> > + char *data = apr_palloc(pool, ++blocksize); /* + space for '\0' */
> >
> > data[0] = '\0';
> >
> > ------------------------------------------------------
> > http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> > d=1189179
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1191456