> -----Original Message----- > From: Stefan Sperling [mailto:s...@elego.de] > Sent: vrijdag 10 september 2010 15:26 > To: Bert Huijben > Cc: dev@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
