You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by iv...@apache.org on 2014/12/25 11:01:38 UTC

svn commit: r1647887 - in /subversion/trunk/subversion: include/mod_dav_svn.h mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c mod_dav_svn/repos.c

Author: ivan
Date: Thu Dec 25 10:01:38 2014
New Revision: 1647887

URL: http://svn.apache.org/r1647887
Log:
Fix unbounded memory usage in mod_authz_svn when SVNPathAuthz short_circuit
option is used.

* subversion/include/mod_dav_svn.h
  (dav_svn_split_uri2, dav_svn_get_repos_path2): New.
  (dav_svn_split_uri, dav_svn_get_repos_path): Update docstring.

* subversion/mod_authz_svn/mod_authz_svn.c
  (get_access_conf): Call dav_svn_get_repos_path2() instead of
   dav_svn_get_repos_path() passing SCRATCH_POOL as POOL parameter.

* subversion/mod_dav_svn/mod_dav_svn.c
  (dav_svn_get_repos_path): Rename to ...
  (dav_svn_get_repos_path2): ... this, adding POOL parameter and use it for
   all allocations.
  (dav_svn_get_repos_path): New, wraps dav_svn_get_repos_path2().

* subversion/mod_dav_svn/repos.c
  (dav_svn_split_uri): Rename to ...
  (dav_svn_split_uri2): ... this, adding POOL parameter and use it for
   all allocations.
  (dav_svn_split_uri): New, wraps dav_svn_split_uri2().

Modified:
    subversion/trunk/subversion/include/mod_dav_svn.h
    subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c
    subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
    subversion/trunk/subversion/mod_dav_svn/repos.c

Modified: subversion/trunk/subversion/include/mod_dav_svn.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/mod_dav_svn.h?rev=1647887&r1=1647886&r2=1647887&view=diff
==============================================================================
--- subversion/trunk/subversion/include/mod_dav_svn.h (original)
+++ subversion/trunk/subversion/include/mod_dav_svn.h Thu Dec 25 10:01:38 2014
@@ -40,7 +40,7 @@ extern "C" {
 /**
    Given an apache request @a r, a @a uri, and a @a root_path to the svn
    location block, process @a uri and return many things, allocated in
-   @a r->pool:
+   @a pool:
 
    - @a cleaned_uri:    The uri with duplicate and trailing slashes removed.
 
@@ -74,7 +74,25 @@ extern "C" {
      - @a relative_path:  /!svn/blah/13/A/B/alpha
      - @a repos_path:     A/B/alpha
      - @a trailing_slash: FALSE
+
+   NOTE: The returned dav_error will be also allocated in @a pool, not
+         in @a r->pool.
+
+   @since New in 1.9
 */
+AP_MODULE_DECLARE(dav_error *) dav_svn_split_uri2(request_rec *r,
+                                                  const char *uri_to_split,
+                                                  const char *root_path,
+                                                  const char **cleaned_uri,
+                                                  int *trailing_slash,
+                                                  const char **repos_basename,
+                                                  const char **relative_path,
+                                                  const char **repos_path,
+                                                  apr_pool_t *pool);
+
+/**
+ * Same as dav_svn_split_uri2() but allocates the result in @a r->pool.
+ */
 AP_MODULE_DECLARE(dav_error *) dav_svn_split_uri(request_rec *r,
                                                  const char *uri,
                                                  const char *root_path,
@@ -87,7 +105,22 @@ AP_MODULE_DECLARE(dav_error *) dav_svn_s
 
 /**
  * Given an apache request @a r and a @a root_path to the svn location
- * block, set @a *repos_path to the path of the repository on disk.  */
+ * block, set @a *repos_path to the path of the repository on disk.
+ * Perform all allocations in @a pool.
+ *
+ * NOTE: The returned dav_error will be also allocated in @a pool, not
+ *       in @a r->pool.
+ *
+ * @since New in 1.9
+ */
+AP_MODULE_DECLARE(dav_error *) dav_svn_get_repos_path2(request_rec *r,
+                                                       const char *root_path,
+                                                       const char **repos_path,
+                                                       apr_pool_t *pool);
+
+/**
+ * Same as dav_svn_get_repos_path2() but allocates the result in@a r->pool.
+ */
 AP_MODULE_DECLARE(dav_error *) dav_svn_get_repos_path(request_rec *r,
                                                       const char *root_path,
                                                       const char **repos_path);

Modified: subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c?rev=1647887&r1=1647886&r2=1647887&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c (original)
+++ subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c Thu Dec 25 10:01:38 2014
@@ -361,7 +361,7 @@ get_access_conf(request_rec *r, authz_sv
   svn_error_t *svn_err = SVN_NO_ERROR;
   dav_error *dav_err;
 
-  dav_err = dav_svn_get_repos_path(r, conf->base_path, &repos_path);
+  dav_err = dav_svn_get_repos_path2(r, conf->base_path, &repos_path, scratch_pool);
   if (dav_err)
     {
       ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "%s", dav_err->desc);

Modified: subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c?rev=1647887&r1=1647886&r2=1647887&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c Thu Dec 25 10:01:38 2014
@@ -662,9 +662,10 @@ dav_svn__get_fs_parent_path(request_rec
 
 
 AP_MODULE_DECLARE(dav_error *)
-dav_svn_get_repos_path(request_rec *r,
-                       const char *root_path,
-                       const char **repos_path)
+dav_svn_get_repos_path2(request_rec *r,
+                        const char *root_path,
+                        const char **repos_path,
+                        apr_pool_t *pool)
 {
 
   const char *fs_path;
@@ -692,19 +693,26 @@ dav_svn_get_repos_path(request_rec *r,
 
   /* Split the svn URI to get the name of the repository below
      the parent path. */
-  derr = dav_svn_split_uri(r, r->uri, root_path,
-                           &ignored_cleaned_uri, &ignored_had_slash,
-                           &repos_name,
-                           &ignored_relative, &ignored_path_in_repos);
+  derr = dav_svn_split_uri2(r, r->uri, root_path,
+                            &ignored_cleaned_uri, &ignored_had_slash,
+                            &repos_name,
+                            &ignored_relative, &ignored_path_in_repos, pool);
   if (derr)
     return derr;
 
   /* Construct the full path from the parent path base directory
      and the repository name. */
-  *repos_path = svn_dirent_join(fs_parent_path, repos_name, r->pool);
+  *repos_path = svn_dirent_join(fs_parent_path, repos_name, pool);
   return NULL;
 }
 
+AP_MODULE_DECLARE(dav_error *)
+dav_svn_get_repos_path(request_rec *r,
+                       const char *root_path,
+                       const char **repos_path)
+{
+  return dav_svn_get_repos_path2(r, root_path, repos_path, r->pool);
+}
 
 const char *
 dav_svn__get_repo_name(request_rec *r)

Modified: subversion/trunk/subversion/mod_dav_svn/repos.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/repos.c?rev=1647887&r1=1647886&r2=1647887&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/repos.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/repos.c Thu Dec 25 10:01:38 2014
@@ -1209,14 +1209,15 @@ static void log_warning(void *baton, svn
 
 
 AP_MODULE_DECLARE(dav_error *)
-dav_svn_split_uri(request_rec *r,
-                  const char *uri_to_split,
-                  const char *root_path,
-                  const char **cleaned_uri,
-                  int *trailing_slash,
-                  const char **repos_basename,
-                  const char **relative_path,
-                  const char **repos_path)
+dav_svn_split_uri2(request_rec *r,
+                   const char *uri_to_split,
+                   const char *root_path,
+                   const char **cleaned_uri,
+                   int *trailing_slash,
+                   const char **repos_basename,
+                   const char **relative_path,
+                   const char **repos_path,
+                   apr_pool_t *pool)
 {
   apr_size_t len1;
   int had_slash;
@@ -1232,7 +1233,7 @@ dav_svn_split_uri(request_rec *r,
   if ((fs_path == NULL) && (fs_parent_path == NULL))
     {
       /* ### are SVN_ERR_APMOD codes within the right numeric space? */
-      return dav_svn__new_error(r->pool, HTTP_INTERNAL_SERVER_ERROR,
+      return dav_svn__new_error(pool, HTTP_INTERNAL_SERVER_ERROR,
                                 SVN_ERR_APMOD_MISSING_PATH_TO_FS,
                                 "The server is misconfigured: "
                                 "either an SVNPath or SVNParentPath "
@@ -1241,7 +1242,7 @@ dav_svn_split_uri(request_rec *r,
     }
 
   /* make a copy so that we can do some work on it */
-  uri = apr_pstrdup(r->pool, uri_to_split);
+  uri = apr_pstrdup(pool, uri_to_split);
 
   /* remove duplicate slashes, and make sure URI has no trailing '/' */
   ap_no2slash(uri);
@@ -1256,7 +1257,7 @@ dav_svn_split_uri(request_rec *r,
     *trailing_slash = FALSE;
 
   /* return the first item.  */
-  *cleaned_uri = apr_pstrdup(r->pool, uri);
+  *cleaned_uri = apr_pstrdup(pool, uri);
 
   /* The URL space defined by the SVN provider is always a virtual
      space. Construct the path relative to the configured Location
@@ -1297,7 +1298,7 @@ dav_svn_split_uri(request_rec *r,
   if (fs_path != NULL)
     {
       /* the repos_basename is the last component of root_path. */
-      *repos_basename = svn_dirent_basename(root_path, r->pool);
+      *repos_basename = svn_dirent_basename(root_path, pool);
 
       /* 'relative' is already correct for SVNPath; the root_path
          already contains the name of the repository, so relative is
@@ -1315,7 +1316,7 @@ dav_svn_split_uri(request_rec *r,
       if (relative[1] == '\0')
         {
           /* ### are SVN_ERR_APMOD codes within the right numeric space? */
-          return dav_svn__new_error(r->pool, HTTP_FORBIDDEN,
+          return dav_svn__new_error(pool, HTTP_FORBIDDEN,
                                     SVN_ERR_APMOD_MALFORMED_URI,
                                     "The URI does not contain the name "
                                     "of a repository.");
@@ -1332,7 +1333,7 @@ dav_svn_split_uri(request_rec *r,
         }
       else
         {
-          magic_component = apr_pstrndup(r->pool, relative + 1,
+          magic_component = apr_pstrndup(pool, relative + 1,
                                          magic_end - relative - 1);
           relative = magic_end;
         }
@@ -1342,7 +1343,7 @@ dav_svn_split_uri(request_rec *r,
     }
 
   /* We can return 'relative' at this point too. */
-  *relative_path = apr_pstrdup(r->pool, relative);
+  *relative_path = apr_pstrdup(pool, relative);
 
   /* Code to remove the !svn junk from the front of the relative path,
      mainly stolen from parse_uri().  This code assumes that
@@ -1363,7 +1364,7 @@ dav_svn_split_uri(request_rec *r,
         if (ch == '\0')
           {
             /* relative is just "!svn", which is malformed. */
-            return dav_svn__new_error(r->pool, HTTP_NOT_FOUND,
+            return dav_svn__new_error(pool, HTTP_NOT_FOUND,
                                       SVN_ERR_APMOD_MALFORMED_URI,
                                       "Nothing follows the svn special_uri.");
           }
@@ -1390,7 +1391,7 @@ dav_svn_split_uri(request_rec *r,
                           *repos_path = NULL;
                         else
                           return dav_svn__new_error(
-                                     r->pool, HTTP_NOT_FOUND,
+                                     pool, HTTP_NOT_FOUND,
                                      SVN_ERR_APMOD_MALFORMED_URI,
                                      "Missing info after special_uri.");
                       }
@@ -1414,7 +1415,7 @@ dav_svn_split_uri(request_rec *r,
                             /* Did we break from the loop prematurely? */
                             if (j != (defn->numcomponents - 1))
                               return dav_svn__new_error(
-                                         r->pool, HTTP_NOT_FOUND,
+                                         pool, HTTP_NOT_FOUND,
                                          SVN_ERR_APMOD_MALFORMED_URI,
                                          "Not enough components after "
                                          "special_uri.");
@@ -1428,13 +1429,13 @@ dav_svn_split_uri(request_rec *r,
                         else
                           {
                             /* Found a slash after the special components. */
-                            *repos_path = apr_pstrdup(r->pool, start - 1);
+                            *repos_path = apr_pstrdup(pool, start - 1);
                           }
                       }
                     else
                       {
                         return
-                          dav_svn__new_error(r->pool, HTTP_NOT_FOUND,
+                          dav_svn__new_error(pool, HTTP_NOT_FOUND,
                                         SVN_ERR_APMOD_MALFORMED_URI,
                                         "Unknown data after special_uri.");
                       }
@@ -1445,7 +1446,7 @@ dav_svn_split_uri(request_rec *r,
 
             if (defn->name == NULL)
               return
-                dav_svn__new_error(r->pool, HTTP_NOT_FOUND,
+                dav_svn__new_error(pool, HTTP_NOT_FOUND,
                                    SVN_ERR_APMOD_MALFORMED_URI,
                                    "Couldn't match subdir after special_uri.");
           }
@@ -1454,13 +1455,27 @@ dav_svn_split_uri(request_rec *r,
       {
         /* There's no "!svn/" at all, so the relative path is already
            a valid path within the repository.  */
-        *repos_path = apr_pstrdup(r->pool, relative - 1);
+        *repos_path = apr_pstrdup(pool, relative - 1);
       }
   }
 
   return NULL;
 }
 
+AP_MODULE_DECLARE(dav_error *)
+dav_svn_split_uri(request_rec *r,
+                  const char *uri_to_split,
+                  const char *root_path,
+                  const char **cleaned_uri,
+                  int *trailing_slash,
+                  const char **repos_basename,
+                  const char **relative_path,
+                  const char **repos_path)
+{
+  return dav_svn_split_uri2(r, uri_to_split, root_path, cleaned_uri,
+                            trailing_slash, repos_basename, relative_path,
+                            repos_path, r->pool);
+}
 
 /* Context for cleanup handler. */
 struct cleanup_fs_access_baton