[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: SVN_PROP_MAX_SIZE (was: Re: svn commit: r1028092 - /subversion/branches/performance/subversion/libsvn_ra_svn/marshal.c)

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Thu, 28 Oct 2010 14:35:49 +0100

On Thu, 2010-10-28 at 14:22 +0200, Stefan Sperling wrote:
> On Thu, Oct 28, 2010 at 09:30:49AM +0200, Erik Huelsmann wrote:
> > >> @@ -593,7 +598,14 @@ static svn_error_t *read_string(svn_ra_s
> > >> svn_ra_svn_item_t *item, apr_uint64_t len)
> > >> {
> > >> svn_stringbuf_t *stringbuf;
> > >> - if (len > 0x100000)
> > >> +
> > >> + /* We should not use large strings in our protocol. However, we may
> > >> + * receive a claim that a very long string is going to follow. In that
> > >> + * case, we start small and wait for all that data to actually show up.
> > >> + * This does not fully prevent DOS attacs but makes them harder (you
> > >> + * have to actually send gigabytes of data).
> > >
> > > Wow, I hadn't even considered this. Once we get this on trunk, it
> > > might make sense to propose a backport, since this has (potential?)
> > > security implications.
> >
> >
> > Actually, that was already released as a security vulnerability some
> > years ago. The comment by Stefan makes it painfully apparent that it
> > is, but I guess that's a good thing. Notice that he did nothing but
> > name the constant and add the explanation. See
> > http://subversion.apache.org/security/CAN-2004-0413-advisory.txt
> >
> > This is exactly the point I was talking about when I said properties
> > are length-limited by ra_svn. (In relation to the maximum size of
> > merge-tracking information.) The actual code is a little bit
> > different than what I remembered., because it does seem to grow the
> > buffer once it gets past the first MiB.
>
> Can we define a constant like SVN_PROP_MAX_SIZE that says how large
> a property can be? This would be useful in the patch code, for instance.
> Right now, a patch with a large property value will cause out of memory
> errors because the code has no way of knowing the maximum size of a
> property value -- so it just keeps reading and allocating.
>
> Also, do we have a plan for what to do when (not "if") mergeinfo in
> repositories out there hits the limit?

Hi Stefan. I thought the issue being addressed here was not imposing a
limit on property size, but rather just limiting the allocation of an
initial chunk of memory which can still be expanded later if the claimed
amount of data actually arrives.

"We should not use large strings in our protocol. However, we may
receive a claim that a very long string is going to follow. In that
case, we start small and wait for all that data to actually show up.
This does not fully prevent DOS attacs but makes them harder (you have
to actually send gigabytes of data)."

"Don't allocate the whole buffer at once ..."

- Julian
Received on 2010-10-28 15:36:32 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.