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

Re: Runaway httpd processes with svn

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 11 Nov 2011 15:21:37 +0100

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

------------------------------------------------------------------------
Received on 2011-11-11 15:22:14 CET

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