Greg Hudson <ghudson@MIT.EDU> writes:
> So now we're relying on dav_svn_convert_err only converting the
> top-level error? That seems like the wrong direction to move in.
Sorry, I just misremembered what svn_error_quick_wrap() does. Rather
odd, considering its name! Here's a new patch:
-*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*- -*-
Fix issue #1501: mod_dav_svn can leak real repository path to client.
* subversion/mod_dav_svn/repos.c
(dav_svn_get_resource): Use one error for the server logs, another
for dav and thence to the client.
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 7837)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -1194,11 +1194,22 @@
serr = svn_repos_open(&(repos->repos), fs_path, r->connection->pool);
if (serr != NULL)
{
- return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- apr_psprintf(r->pool,
- "Could not open the SVN "
- "filesystem at %s",
- fs_path));
+ /* The error returned by svn_repos_open might contain the
+ actual path to the failed repository. We don't want to
+ leak that path back to the client, because that would be
+ a security risk, but we do want to log the real error on
+ the server side. */
+ const char *new_msg = "Could not open the requested SVN filesystem";
+ svn_error_t *sanitized_error = svn_error_create(serr->apr_err,
+ NULL, new_msg);
+
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r,
+ "%s", serr->message);
+
+ /* Return a slightly less informative error to dav. */
+ return dav_svn_convert_err (sanitized_error,
+ HTTP_INTERNAL_SERVER_ERROR,
+ apr_psprintf(r->pool, new_msg));
}
/* Cache the open repos for the next request on this connection */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 24 21:58:39 2003