On Fri, Nov 11, 2011 at 01:45:52PM -0000, Echlin, Jamie wrote:
> Hi Stefan,
>
> > > Could short-circuit be implicated here?
> >
> > Yes. The trace shows that you're falling into the
> > short-circuit code path. Try disabling the option to see if
> > the problem persists.
> >
> > Which version of Subversion are you running on the server?
>
> 1.6.12, so within the range of versions affected.
>
> We could disable the option but as we don't know which repo is causing
> it we'd have to do it on all, and this badly affects performance.
>
> I could not find the patch mentioned on the advisory, so it maybe that
> we upgrade to 1.6.17 asap.
>
Here's the 1.6.x patch, extracted from the revision log.
You don't strictly need the parts that touch tests/cmdline/.
------------------------------------------------------------------------
r1130551 | cmpilato | 2011-06-02 15:52:49 +0200 (Thu, 02 Jun 2011) | 9 lines
Merge from trunk r1130303.
* r1130303
CVE-2011-1921 and CVE-2011-1783.
Justification:
Already released in 1.6.17.
Votes:
+3: cmpilato (I'm applying the "obvious fix" rule)
Index: subversion/mod_dav_svn/authz.c
===================================================================
--- subversion/mod_dav_svn/authz.c (revision 1130550)
+++ subversion/mod_dav_svn/authz.c (revision 1130551)
@@ -46,6 +46,11 @@ dav_svn__allow_read(request_rec *r,
return TRUE;
}
+ /* Sometimes we get paths that do not start with '/' and
+ hence below uri concatenation would lead to wrong uris .*/
+ if (path && path[0] != '/')
+ path = apr_pstrcat(pool, "/", path, NULL);
+
/* If bypass is specified and authz has exported the provider.
Otherwise, we fall through to the full version. This should be
safer than allowing or disallowing all accesses if there is a
Index: subversion/tests/cmdline/svnsync_tests.py
===================================================================
--- subversion/tests/cmdline/svnsync_tests.py (revision 1130550)
+++ subversion/tests/cmdline/svnsync_tests.py (revision 1130551)
@@ -802,6 +802,66 @@ def descend_into_replace(sbox):
run_test(sbox, "descend_into_replace.dump", subdir='/trunk/H',
exp_dump_file_name = "descend_into_replace.expected.dump")
+def specific_deny_authz(sbox):
+ "verify if specifically denied paths dont sync"
+
+ sbox.build("specific-deny-authz")
+
+ dest_sbox = sbox.clone_dependent()
+ build_repos(dest_sbox)
+
+ svntest.actions.enable_revprop_changes(dest_sbox.repo_dir)
+
+ run_init(dest_sbox.repo_url, sbox.repo_url)
+
+ svntest.main.run_svn(None, "cp",
+ os.path.join(sbox.wc_dir, "A"),
+ os.path.join(sbox.wc_dir, "A_COPY")
+ )
+ svntest.main.run_svn(None, "ci", "-mm", sbox.wc_dir)
+
+ write_restrictive_svnserve_conf(sbox.repo_dir)
+
+ # For mod_dav_svn's parent path setup we need per-repos permissions in
+ # the authz file...
+ if sbox.repo_url.startswith('http'):
+ svntest.main.file_write(sbox.authz_file,
+ "[specific-deny-authz:/]\n"
+ "* = r\n"
+ "\n"
+ "[specific-deny-authz:/A]\n"
+ "* = \n"
+ "\n"
+ "[specific-deny-authz:/A_COPY/B/lambda]\n"
+ "* = \n"
+ "\n"
+ "[specific-deny-authz-1:/]\n"
+ "* = rw\n")
+ # Otherwise we can just go with the permissions needed for the source
+ # repository.
+ else:
+ svntest.main.file_write(sbox.authz_file,
+ "[/]\n"
+ "* = r\n"
+ "\n"
+ "[/A]\n"
+ "* = \n"
+ "\n"
+ "[/A_COPY/B/lambda]\n"
+ "* = \n")
+
+ run_sync(dest_sbox.repo_url)
+
+ lambda_url = dest_sbox.repo_url + '/A_COPY/B/lambda'
+
+ # this file should have been blocked by authz
+ svntest.actions.run_and_verify_svn(None,
+ [], svntest.verify.AnyOutput,
+ 'cat',
+ lambda_url)
+
+
+
########################################################################
# Run the tests
@@ -841,6 +901,7 @@ test_list = [ None,
delete_svn_props,
commit_a_copy_of_root,
descend_into_replace,
+ Skip(specific_deny_authz, svntest.main.is_ra_type_file),
]
if __name__ == '__main__':
Index: subversion/libsvn_repos/authz.c
===================================================================
--- subversion/libsvn_repos/authz.c (revision 1130550)
+++ subversion/libsvn_repos/authz.c (revision 1130551)
@@ -746,6 +746,9 @@ svn_repos_authz_check_access(svn_authz_t *authz, c
return SVN_NO_ERROR;
}
+ /* Sanity check. */
+ SVN_ERR_ASSERT(path[0] == '/');
+
/* Determine the granted access for the requested path. */
while (!authz_get_path_access(authz->cfg, repos_name,
current_path, user,
Index: .
===================================================================
--- . (revision 1130550)
+++ . (revision 1130551)
Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
Merged /subversion/trunk:r1130303
------------------------------------------------------------------------
