> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: vrijdag 10 september 2010 15:26
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org
> Subject: Re: svn commit: r995475 -
> /subversion/trunk/subversion/libsvn_repos/load.c
>
> On Thu, Sep 09, 2010 at 08:58:10PM +0200, Bert Huijben wrote:
> > > @@ -524,9 +524,11 @@ parse_property_block(svn_stream_t *strea
> > > else if ((buf[0] == 'D') && (buf[1] == ' '))
> > > {
> > > char *keybuf;
> > > + apr_int64_t len;
> > >
> > > + SVN_ERR(svn_cstring_atoi64(&len, buf + 2));
> > > SVN_ERR(read_key_or_val(&keybuf, actual_length,
> > > - stream, atoi(buf + 2), proppool));
> > > + stream, (apr_size_t)len,
proppool));
> >
> > What would this do if you have 4GB + 1 byte?
> >
> > I expected that we would use the new svn_cstring conversion variants
> > to check for that kind of errors (for overflows, etc.), but now we
> > just ignore the error at a different level.
>
> How so? apr_strtoi64() will set errno upon overflow so we'll error out.
> atoi() did no such thing.
Yes, after 2^64 apr_stroi64() will set an error. But you truncate it to 2^32
in the read_key_or_val() line. (You explicitly suppressed the warning for
this problem)
So instead of erroring at 2^32 , you just created a different problem, with
possible different security implications.
I think we need a special svn_string_atosize() (Beter name welcome) to
compensate for this problem.
Bert
Received on 2010-09-10 16:49:07 CEST