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

Re: reparent bug in svnserve

From: David Glasser <glasser_at_mit.edu>
Date: 2006-11-26 16:42:47 CET

On 11/26/06, David Glasser <glasser@mit.edu> wrote:
> On 11/23/06, Alexander Sinyushkin <Alexander.Sinyushkin@svnkit.com> wrote:
> > Hello, guys, seems that there's a bug in svnserve's 'reparent'
> > behaviour. The point is that svnserve sends 'success' on a try to
> > reparent to a different repository although documentation says the
> > opposite. The problem appears because the 'get_fs_path' function in
> > 'serve.c' checks only the first n bytes of two urls, where n - is
> > the length of the old repos_root, but a new url may contain all the
> > previous bytes as the old one does, however at the same time it may
> > be not the same repository. For example:
> > old_repos_url = "svn://localhost/svn-test-work/repositories/externals_tests-1"
> > new_url = "svn://localhost/svn-test-work/repositories/externals_tests-1.other"
> >
> > This is what I noticed while running 'externals' python tests.
>
> Good point, Alexander. What do you think the fix is -- checking to
> see if the next character after the matching bit is '\0' or '/'? (Are
> we guaranteed that it uses forward slashes in this part of the code?)
>
> On the other hand, the situation in the other ra modules seems to be
> even worse: none of the other three do any checks, and it looks like
> the ra_local one could even manage to pass around pointers to
> unallocated memory.

Actually, hmm, I think I see what the real issue is. Turns out that
unlike most of the stubs in libsvn_ra/ra_loader.c, svn_ra_reparent
actually does some common work --- it calls svn_ra_get_repos_root and
does the appropriate check with svn_path_is_ancestor. So what you
describe should not be a problem for anything that actually uses the C
client implementation of svn_ra_reparent. On the other hand, if there
in theory (wink wink) existed, say, a Java reimplementation that tried
to speak the same protocol without making that client-side check
first, then sure, I see that there could be trouble.

I'd recommend that SVNKit make the same client-side check as the C
libraries do, but we should still probably fix this (using
svn_path_is_ancestor). (Plus, there could hypothetically be a race
condition or something with the client-side check.)

--dave

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 26 16:43:02 2006

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