mod_dontdothat does not restrict requests from serf based clients. Summary: ======== mod_dontdothat allows you to block update REPORT requests against certain paths in the repository. It expects the paths in the REPORT request to be absolute URLs. Serf based clients send relative URLs instead of absolute URLs in many cases. As a result these clients are not blocked as configured by mod_dontdothat. Known vulnerable: ================= mod_dontdothat 1.4.0 through 1.7.13 mod_dontdothat 1.8.0 through 1.8.4 Note that mod_dontdothat was in contrib until 1.7.3 and contrib is not included in Subversion source tarballs since 1.7.0, so Subversion 1.7.0 through 1.7.2 did not included mod_dontdothat (it was still available from the repository tags for those versions under contrib). Known fixed: ============ mod_dontdothat 1.7.14 mod_dontdothat 1.8.5 Details: ======== mod_dontdothat allows the blocking of certain update REPORT requests based on the paths of the requests. This is typically done to block requests against the root of the repository or the tags and branches directories where there may be large trees and require a large amount of server resources to fulfill. Update REPORT requests are used to fulfill requests from the client for the following commands: checkout update export diff (when a server URL or revision other than the BASE is specified) status -u copy $URL $WC The request body for the request includes a src-path and sometimes a dst-path entity. mod_dontdothat matches those paths against the configured paths to deny. When matching the src-path and dst-path, mod_dontdothat expects that an absolute URL will be provided. However, serf clients in the case of the src-path entity only provided a relative path. Relative paths have been supported by mod_dav_svn since before Subversion 1.0, but neon based clients never produced them. When a path is not an absolute URL then mod_dontdothat allowed the request. As a result a serf client was not blocked by mod_dontdothat. It's possible for other clients to be modified to avoid the restrictions as well, though we are unaware of anyone doing so. Severity: ========= CVSSv2 Base Score: 2.6 CVSSv2 Base Vector: AV:N/AC:H/Au:N/C:N/I:N/A:P We consider this to be a low risk vulnerability. mod_dontdothat is not typically installed. It is not intended or useful as an access control mechanism. Rather it exists primarily to prevent users unintentionally making expensive requests against the server. Clients may be able to use more resources than the server admin may have expected and planned for based on their configuration. This increased resource usage may impact performance and the availability of the server. A server admin who has configured mod_dontdothat would expect matching update REPORT requests to be blocked, but they will not be with serf based clients. Serf was added as a http library in Subversion 1.4 as a compile time option. In 1.5 it was possible to chose it at run time, provided it had been enabled at compile time. With 1.8 it became the only supported http library. As a result clients that can evade these restrictions are in common use and no special effort is required to do so. Recommendations: ================ Admins using mod_dontdothat are advised to upgrade to 1.7.14 or 1.8.5. It may be possible to configure http to disable all requests without an absolute URL in the update REPORT requests. However, doing so has the effect of disabling all serf based clients. Given that serf is the only http library for 1.8.x we do not recommend doing so. References: =========== CVE-2013-4505 (Subversion) Reported by: ============ Ben Reser, WANdisco Patches: ======== Patch for Subversion 1.7.x and 1.8.x: [[[ Index: tools/server-side/mod_dontdothat/mod_dontdothat.c =================================================================== --- tools/server-side/mod_dontdothat/mod_dontdothat.c (revision 1541183) +++ tools/server-side/mod_dontdothat/mod_dontdothat.c (working copy) @@ -30,6 +30,7 @@ #include <util_filter.h> #include <ap_config.h> #include <apr_strings.h> +#include <apr_uri.h> #include <expat.h> @@ -36,6 +37,8 @@ #include "mod_dav_svn.h" #include "svn_string.h" #include "svn_config.h" +#include "svn_path.h" +#include "private/svn_fspath.h" module AP_MODULE_DECLARE_DATA dontdothat_module; @@ -161,6 +164,34 @@ } } +/* duplicate of dav_svn__log_err() from mod_dav_svn/util.c */ +static void +log_dav_err(request_rec *r, + dav_error *err, + int level) +{ + dav_error *errscan; + + /* Log the errors */ + /* ### should have a directive to log the first or all */ + for (errscan = err; errscan != NULL; errscan = errscan->prev) { + apr_status_t status; + + if (errscan->desc == NULL) + continue; + +#if AP_MODULE_MAGIC_AT_LEAST(20091119,0) + status = errscan->aprerr; +#else + status = errscan->save_errno; +#endif + + ap_log_rerror(APLOG_MARK, level, status, r, + "%s [%d, #%d]", + errscan->desc, errscan->status, errscan->error_id); + } +} + static svn_boolean_t is_this_legal(dontdothat_filter_ctx *ctx, const char *uri) { @@ -167,20 +198,37 @@ const char *relative_path; const char *cleaned_uri; const char *repos_name; + const char *uri_path; int trailing_slash; dav_error *derr; - /* Ok, so we need to skip past the scheme, host, etc. */ - uri = ap_strstr_c(uri, "://"); - if (uri) - uri = ap_strchr_c(uri + 3, '/'); + /* uri can be an absolute uri or just a path, we only want the path to match + * against */ + if (uri && svn_path_is_url(uri)) + { + apr_uri_t parsed_uri; + apr_status_t rv = apr_uri_parse(ctx->r->pool, uri, &parsed_uri); + if (APR_SUCCESS != rv) + { + /* Error parsing the URI, log and reject request. */ + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, ctx->r, + "mod_dontdothat: blocked request after failing " + "to parse uri: '%s'", uri); + return FALSE; + } + uri_path = parsed_uri.path; + } + else + { + uri_path = uri; + } - if (uri) + if (uri_path) { const char *repos_path; derr = dav_svn_split_uri(ctx->r, - uri, + uri_path, ctx->cfg->base_path, &cleaned_uri, &trailing_slash, @@ -194,7 +242,7 @@ if (! repos_path) repos_path = ""; - repos_path = apr_psprintf(ctx->r->pool, "/%s", repos_path); + repos_path = svn_fspath__canonicalize(repos_path, ctx->r->pool); /* First check the special cases that are always legal... */ for (idx = 0; idx < ctx->allow_recursive_ops->nelts; ++idx) @@ -228,7 +276,20 @@ } } } + else + { + log_dav_err(ctx->r, derr, APLOG_ERR); + return FALSE; + } + } + else + { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, + "mod_dontdothat: empty uri passed to is_this_legal(), " + "module bug?"); + return FALSE; + } return TRUE; } ]]]