Re: svn commit: r1366209 - in /subversion/trunk/subversion: libsvn_wc/externals.c tests/libsvn_client/client-test.c

[Blair Zajac <bl...@orcaware.com>]

On 07/26/2012 03:04 PM, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Thu Jul 26 22:04:03 2012
> New Revision: 1366209
>
> URL: http://svn.apache.org/viewvc?rev=1366209&view=rev
> Log:
> * subversion/libsvn_wc/externals.c
>    (svn_wc__resolve_relative_external_url):
>      Deny /../ syntax in urls in externals. Stepping over the
>      root of a server is not possible.

The first sentence sounds like no /../ are allowed in any external URLs 
which isn't the case, it appears with your change they are not allowed 
anywhere for any scheme or server root relative path, just just past the 
first two characters.

Blair

RE: svn commit: r1366209 - in /subversion/trunk/subversion: libsvn_wc/externals.c tests/libsvn_client/client-test.c

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: Blair Zajac [mailto:blair@orcaware.com]
> Sent: vrijdag 27 juli 2012 01:47
> To: rhuijben@apache.org
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1366209 - in /subversion/trunk/subversion:
> libsvn_wc/externals.c tests/libsvn_client/client-test.c
> 
> On 07/26/2012 03:04 PM, rhuijben@apache.org wrote:
> > Author: rhuijben
> > Date: Thu Jul 26 22:04:03 2012
> > New Revision: 1366209
> >
> > URL: http://svn.apache.org/viewvc?rev=1366209&view=rev
> > Log:
> > * subversion/libsvn_wc/externals.c
> >    (svn_wc__resolve_relative_external_url):
> >      Deny /../ syntax in urls in externals. Stepping over the
> >      root of a server is not possible.
> 
> The first sentence sounds like no /../ are allowed in any external URLs
> which isn't the case, it appears with your change they are not allowed
> anywhere for any scheme or server root relative path, just just past the
> first two characters.

Hmm, not sure how to put it in a log message, but this is specifically about
/../something/style
relative paths.

We already denied
//../url

And 
/url/../../path

While we (via a different code path) do allow ../../../some/dir and ^/../../some/dir

The +2 which I removed with my patch was originally added to allow the // and ^/ paths to skip the relpath rules. But since they now use a different code path I re-enabled the original check to disallow /../

With the specific /../something/style url we would generate
http://svn.apache.org/../something/style (assuming a current repository of http://svn.apache.org/repos/asf)
urls, which should (as far as I can tell) never work and are certainly not recommended.

	Bert