From: Eric Covener Date: Wed, 5 Jun 2013 15:10:43 +0000 (+0000) Subject: Backport AllowAnyURI related revisions from 2.2.x to pave the way for CVE-2011-4317 X-Git-Tag: 2.0.65~29 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=248315395f2ea37396aa24673951361e325260af;p=thirdparty%2Fapache%2Fhttpd.git Backport AllowAnyURI related revisions from 2.2.x to pave the way for CVE-2011-4317 http://svn.apache.org/viewvc?rev=1375113&view=rev http://svn.apache.org/viewvc?rev=1447508&view=rev Reviewed by:rjung, wrowe, covener git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@1489910 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 00f14fdadd4..986fd141643 100644 --- a/CHANGES +++ b/CHANGES @@ -28,6 +28,16 @@ Changes with Apache 2.0.65 is enabled, could allow local users to gain privileges via a .htaccess file. [Stefan Fritsch, Greg Ames] + *) mod_rewrite: Allow merging RewriteBase down to subdirectories + if new option 'RewriteOptions MergeBase' is configured. + [Eric Covener] + + *) mod_rewrite: Fix the RewriteEngine directive to work within a + location. Previously, once RewriteEngine was switched on globally, + it was impossible to switch off. [Graham Leggett] + + *) mod_rewrite: Add "AllowAnyURI" option. PR 52774. [Joe Orton] + *) htdigest: Fix buffer overflow when reading digest password file with very long lines. PR 54893. [Rainer Jung] diff --git a/STATUS b/STATUS index 034575281d4..efb976df0ab 100644 --- a/STATUS +++ b/STATUS @@ -164,29 +164,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * Add AllowAnyURI, fix mod_rewrite configuration in Location. - Patch must be applied on top of the CVE-2011-4317 patch above. - Note that I added a minor MMN bump, since in 2.0 the structure definitions - are in mod_rewrite.h and not in mod_rewrite.c, so the needed change IMHO - is public and needs a bump. - trunk patch: http://svn.apache.org/viewvc?view=revision&revision=1356115 and - http://svn.apache.org/viewvc?view=revision&revision=1356813 and - http://svn.apache.org/viewvc?view=revision&revision=1086662 and - http://svn.apache.org/viewvc?view=revision&revision=1032431 and - http://svn.apache.org/viewvc?view=revision&revision=1410681 and - http://svn.apache.org/viewvc?view=revision&revision=1447426 - 2.4.x patch: http://svn.apache.org/viewvc?view=revision&revision=1359687 and - http://svn.apache.org/viewvc?view=revision&revision=1086662 and - http://svn.apache.org/viewvc?view=revision&revision=1032431 and - http://svn.apache.org/viewvc?view=revision&revision=1418954 and - http://svn.apache.org/viewvc?view=revision&revision=1447448 - 2.2.x patch: http://svn.apache.org/viewvc?rev=1375113&view=rev and - http://svn.apache.org/viewvc?rev=1447508&view=rev - 2.0.x patch: http://people.apache.org/~rjung/patches/2.0-AllowAnyURI-v3.patch - +1: rjung, wrowe, covener - rjung: I backported the MergeBase option plus no merging as default form 2.2. - 2.0-AllowAnyURI-v3.patch contains that now. - * byterange: Backport MaxRanges configuration directive and ap_set_accept_ranges() utility function. Trunk patch: http://svn.apache.org/viewvc?view=revision&revision=1162584 diff --git a/docs/manual/mod/mod_rewrite.xml b/docs/manual/mod/mod_rewrite.xml index a2e90b0e198..fdeb26e8b12 100644 --- a/docs/manual/mod/mod_rewrite.xml +++ b/docs/manual/mod/mod_rewrite.xml @@ -271,7 +271,50 @@ later responds with an 500 Internal Server Error. If you really need more internal redirects than 10 per request, you may increase the default to the desired value. + +
AllowAnyURI
+
+ +

When RewriteRule + is used in VirtualHost or server context with + version 2.0.65 or later of httpd, mod_rewrite + will only process the rewrite rules if the request URI is a URL-path. This avoids + some security issues where particular rules could allow + "surprising" pattern expansions (see CVE-2011-3368 + and CVE-2011-4317). + To lift the restriction on matching a URL-path, the + AllowAnyURI option can be enabled, and + mod_rewrite will apply the rule set to any + request URI string, regardless of whether that string matches + the URL-path grammar required by the HTTP specification.

+ + + Security Warning + +

Enabling this option will make the server vulnerable to + security issues if used with rewrite rules which are not + carefully authored. It is strongly recommended + that this option is not used. In particular, beware of input + strings containing the '@' character which could + change the interpretation of the transformed URI, as per the + above CVE names.

+ +
+ +
MergeBase
+
+ +

With this option, the value of RewriteBase is copied from where it's explicitly defined + into any sub-directory or sub-location that doesn't define its own + RewriteBase. + This flag is available for Apache HTTP Server 2.0.65 and later.

+
+ diff --git a/include/ap_mmn.h b/include/ap_mmn.h index a370480316a..6d8458f65dc 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -88,6 +88,8 @@ * 20020903.12 (2.0.56-dev) added ap_get_server_revision / ap_version_t * 20020903.13 (2.0.62-dev) Add *ftp_directory_charset to proxy_dir_conf * 20020903.14 (2.0.64-dev) added ap_vhost_iterate_given_conn + * 20020903.15 (2.0.65-dev) added state_set, options_set, baseurl_set to + * rewrite_server_conf and rewrite_perdir_conf */ #define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */ @@ -95,7 +97,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20020903 #endif -#define MODULE_MAGIC_NUMBER_MINOR 14 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 15 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c index e26bb6ec30a..23c9e96813f 100644 --- a/modules/mappers/mod_rewrite.c +++ b/modules/mappers/mod_rewrite.c @@ -191,8 +191,11 @@ static void *config_server_merge(apr_pool_t *p, void *basev, void *overridesv) base = (rewrite_server_conf *)basev; overrides = (rewrite_server_conf *)overridesv; - a->state = overrides->state; - a->options = overrides->options; + a->state = (overrides->state_set == 0) ? base->state : overrides->state; + a->state_set = overrides->state_set || base->state_set; + a->options = (overrides->options_set == 0) ? base->options : overrides->options; + a->options_set = overrides->options_set || base->options_set; + a->server = overrides->server; a->redirect_limit = overrides->redirect_limit ? overrides->redirect_limit @@ -280,13 +283,23 @@ static void *config_perdir_merge(apr_pool_t *p, void *basev, void *overridesv) base = (rewrite_perdir_conf *)basev; overrides = (rewrite_perdir_conf *)overridesv; - a->state = overrides->state; - a->options = overrides->options; - a->directory = overrides->directory; - a->baseurl = overrides->baseurl; + a->state = (overrides->state_set == 0) ? base->state : overrides->state; + a->state_set = overrides->state_set || base->state_set; + a->options = (overrides->options_set == 0) ? base->options : overrides->options; + a->options_set = overrides->options_set || base->options_set; a->redirect_limit = overrides->redirect_limit ? overrides->redirect_limit : base->redirect_limit; + + if (a->options & OPTION_MERGEBASE) { + a->baseurl = (overrides->baseurl_set == 0) ? base->baseurl : overrides->baseurl; + a->baseurl_set = overrides->baseurl_set || base->baseurl_set; + } + else { + a->baseurl = overrides->baseurl; + } + + a->directory = overrides->directory; if (a->options & OPTION_INHERIT) { a->rewriteconds = apr_array_append(p, overrides->rewriteconds, @@ -317,11 +330,17 @@ static const char *cmd_rewriteengine(cmd_parms *cmd, sconf = ap_get_module_config(cmd->server->module_config, &rewrite_module); - if (cmd->path == NULL) { /* is server command */ + /* server command? set both global scope and base directory scope */ + if (cmd->path == NULL) { sconf->state = (flag ? ENGINE_ENABLED : ENGINE_DISABLED); + sconf->state_set = 1; + dconf->state = sconf->state; + dconf->state_set = 1; } - else /* is per-directory command */ { + /* directory command? set directory scope only */ + else { dconf->state = (flag ? ENGINE_ENABLED : ENGINE_DISABLED); + dconf->state_set = 1; } return NULL; @@ -350,28 +369,41 @@ static const char *cmd_rewriteoptions(cmd_parms *cmd, return "RewriteOptions: MaxRedirects has the format MaxRedirects" "=n."; } + else if (!strcasecmp(w, "allowanyuri")) { + options |= OPTION_ANYURI; + } + else if (!strcasecmp(w, "mergebase")) { + options |= OPTION_MERGEBASE; + } else { return apr_pstrcat(cmd->pool, "RewriteOptions: unknown option '", w, "'", NULL); } } - /* put it into the appropriate config */ + /* server command? set both global scope and base directory scope */ if (cmd->path == NULL) { /* is server command */ - rewrite_server_conf *conf = + rewrite_perdir_conf *dconf = in_dconf; + rewrite_server_conf *sconf = ap_get_module_config(cmd->server->module_config, &rewrite_module); - conf->options |= options; - conf->redirect_limit = limit; - } + sconf->options |= options; + sconf->options_set = 1; + sconf->redirect_limit = limit; + dconf->options |= options; + dconf->options_set = 1; + dconf->redirect_limit = limit; + } + /* directory command? set directory scope only */ else { /* is per-directory command */ - rewrite_perdir_conf *conf = in_dconf; + rewrite_perdir_conf *dconf = in_dconf; - conf->options |= options; - conf->redirect_limit = limit; + dconf->options |= options; + dconf->options_set = 1; + dconf->redirect_limit = limit; } - + return NULL; } @@ -541,6 +573,7 @@ static const char *cmd_rewritebase(cmd_parms *cmd, void *in_dconf, } dconf->baseurl = a1; + dconf->baseurl_set = 1; return NULL; } @@ -1090,6 +1123,7 @@ static void init_child(apr_pool_t *p, server_rec *s) static int hook_uri2file(request_rec *r) { + rewrite_perdir_conf *dconf; rewrite_server_conf *conf; const char *saved_rulestatus; const char *var; @@ -1109,11 +1143,14 @@ static int hook_uri2file(request_rec *r) */ conf = ap_get_module_config(r->server->module_config, &rewrite_module); + dconf = (rewrite_perdir_conf *)ap_get_module_config(r->per_dir_config, + &rewrite_module); + /* * only do something under runtime if the engine is really enabled, * else return immediately! */ - if (conf->state == ENGINE_DISABLED) { + if (!dconf || dconf->state == ENGINE_DISABLED) { return DECLINED; } @@ -1129,6 +1166,19 @@ static int hook_uri2file(request_rec *r) return DECLINED; } + /* Unless the anyuri option is set, ensure that the input to the + * first rule really is a URL-path, avoiding security issues with + * poorly configured rules. See CVE-2011-3368, CVE-2011-4317. */ + if ((dconf->options & OPTION_ANYURI) == 0 + && ((r->unparsed_uri[0] == '*' && r->unparsed_uri[1] == '\0') + || !r->uri || r->uri[0] != '/')) { + rewritelog(r, 8, "Declining, request-URI '%s' is not a URL-path. " + "Consult the manual entry for the RewriteOptions directive " + "for options and caveats about matching other strings.", + r->uri); + return DECLINED; + } + /* * add the SCRIPT_URL variable to the env. this is a bit complicated * due to the fact that apache uses subrequests and internal redirects @@ -1457,7 +1507,7 @@ static int hook_fixup(request_rec *r) * only do something under runtime if the engine is really enabled, * for this directory, else return immediately! */ - if (dconf->state == ENGINE_DISABLED) { + if (!dconf || dconf->state == ENGINE_DISABLED) { return DECLINED; } diff --git a/modules/mappers/mod_rewrite.h b/modules/mappers/mod_rewrite.h index e648da98a83..95f3201cd4b 100644 --- a/modules/mappers/mod_rewrite.h +++ b/modules/mappers/mod_rewrite.h @@ -138,6 +138,8 @@ #define OPTION_NONE 1<<0 #define OPTION_INHERIT 1<<1 +#define OPTION_ANYURI 1<<4 +#define OPTION_MERGEBASE 1<<5 #define CACHEMODE_TS 1<<0 #define CACHEMODE_TTL 1<<1 @@ -231,6 +233,8 @@ typedef struct { apr_array_header_t *rewriterules; /* the RewriteRule entries */ server_rec *server; /* the corresponding server indicator */ int redirect_limit; /* maximum number of internal redirects */ + unsigned int state_set:1; + unsigned int options_set:1; } rewrite_server_conf; @@ -245,6 +249,9 @@ typedef struct { char *directory; /* the directory where it applies */ const char *baseurl; /* the base-URL where it applies */ int redirect_limit; /* maximum number of internal redirects */ + unsigned int state_set:1; + unsigned int options_set:1; + unsigned int baseurl_set:1; } rewrite_perdir_conf; diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c index 84d5fb10bda..4b2715a5a17 100644 --- a/modules/proxy/mod_proxy.c +++ b/modules/proxy/mod_proxy.c @@ -140,6 +140,11 @@ static int proxy_trans(request_rec *r) return OK; } + if ((r->unparsed_uri[0] == '*' && r->unparsed_uri[1] == '\0') + || !r->uri || r->uri[0] != '/') { + return DECLINED; + } + /* XXX: since r->uri has been manipulated already we're not really * compliant with RFC1945 at this point. But this probably isn't * an issue because this is a hybrid proxy/origin server. diff --git a/server/protocol.c b/server/protocol.c index 9b05c6539f3..c263c4f61e6 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -628,25 +628,6 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb) ap_parse_uri(r, uri); - /* RFC 2616: - * Request-URI = "*" | absoluteURI | abs_path | authority - * - * authority is a special case for CONNECT. If the request is not - * using CONNECT, and the parsed URI does not have scheme, and - * it does not begin with '/', and it is not '*', then, fail - * and give a 400 response. */ - if (r->method_number != M_CONNECT - && !r->parsed_uri.scheme - && uri[0] != '/' - && !(uri[0] == '*' && uri[1] == '\0')) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "invalid request-URI %s", uri); - r->args = NULL; - r->hostname = NULL; - r->status = HTTP_BAD_REQUEST; - r->uri = apr_pstrdup(r->pool, uri); - } - if (ll[0]) { r->assbackwards = 0; pro = ll;