Received: (qmail 67095 invoked by uid 501); 13 Oct 2000 18:33:16 -0000 Message-Id: <20001013183316.67094.qmail@locus.apache.org> Date: 13 Oct 2000 18:33:16 -0000 From: Anders Henke Reply-To: anders@schlund.de To: submit@bugz.apache.org Subject: backreferences aren't being expanded for internal RewriteMaps X-Send-Pr-Version: 3.110 >Number: 6671 >Category: mod_rewrite >Synopsis: backreferences aren't being expanded for internal RewriteMaps >Confidential: no >Severity: serious >Priority: medium >Responsible: apache >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: apache >Arrival-Date: Fri Oct 13 11:40:01 PDT 2000 >Closed-Date: Thu Oct 26 23:10:22 PDT 2000 >Last-Modified: Thu Oct 26 23:10:22 PDT 2000 >Originator: anders@schlund.de >Release: 1.3.14 >Organization: >Environment: Linux 2.2.17, gcc 2.7.2.3, plain Apache 1.3.14 >Description: We've written an own internal RewriteMap for mass virtual hosting set up as following: RewriteMap ourmapping int:ourmap RewriteRule (.*) ${ourmapping:$1} Apache 1.3.9 correctly hands the (.*)'s value to the map, Apache 1.3.14 hands over the given backreferences name '$1'. >How-To-Repeat: RewriteMap test int:toupper RewriteRule (.*) ${test:$1} should rewrite every URL to its uppercase counterpart; instead, Apache hands the $1 over to the internal RewriteMap, which again outputs the $1. Apache takes the given $1 as its new URL and gives a 404. >Fix: Fix it :-) >Release-Note: >Audit-Trail: Comment-Added-By: slive Comment-Added-When: Sun Oct 15 21:06:35 PDT 2000 Comment-Added: This seems to be a result of recent security changes in the mod_rewrite code. There is no fix available yet, but I have reported the problem to the development discussion list. Comment-Added-By: slive Comment-Added-When: Mon Oct 16 22:25:28 PDT 2000 Comment-Added: The following is just a quick hack which restores part of the pre-1.3.14 behaviour. It works for me, but it has been hardly tested, so use it at your own risk. --- mod_rewrite.c.old Mon Oct 16 22:17:22 2000 +++ mod_rewrite.c Mon Oct 16 22:10:06 2000 @@ -2217,6 +2217,7 @@ ** +-------------------------------------------------------+ */ +static void expand_map_lookups(request_rec *r, char *uri, int uri_len); /* ** @@ -2256,42 +2257,15 @@ break; } /* now we have a '$' or a '%' */ - if (inp[1] == '{') { + if (inp[0] == '%' && inp[1] == '{') { char *endp; endp = strchr(inp, '}'); if (endp == NULL) { goto skip; } *endp = '\0'; - if (inp[0] == '$') { - /* ${...} map lookup expansion */ - char *key, *dflt, *result; - key = strchr(inp, ':'); - if (key == NULL) { - goto skip; - } - *key++ = '\0'; - dflt = strchr(key, '|'); - if (dflt) { - *dflt++ = '\0'; - } - result = lookup_map(r, inp+2, key); - if (result == NULL) { - result = dflt ? dflt : ""; - } - span = ap_cpystrn(outp, result, space) - outp; - key[-1] = ':'; - if (dflt) { - dflt[-1] = '|'; - } - } - else if (inp[0] == '%') { - /* %{...} variable lookup expansion */ - span = ap_cpystrn(outp, lookup_variable(r, inp+2), space) - outp; - } - else { - span = 0; - } + /* %{...} variable lookup expansion */ + span = ap_cpystrn(outp, lookup_variable(r, inp+2), space) - outp; *endp = '}'; inp = endp+1; outp += span; @@ -2328,7 +2302,128 @@ space--; } *outp++ = '\0'; + + /* Expand ${...} (RewriteMap lookups) */ + expand_map_lookups(r,buffer,nbuf); + } + +/* +** +** mapfile expansion support +** i.e. expansion of MAP lookup directives +** ${:} in RewriteRule rhs +** +*/ + + #define limit_length(n) (n > LONG_STRING_LEN-1 ? LONG_STRING_LEN-1 : n) + +static void expand_map_lookups(request_rec *r, char *uri, int uri_len) +{ + char newuri[MAX_STRING_LEN]; + char *cpI; + char *cpIE; + char *cpO; + char *cpT; + char *cpT2; + char mapname[LONG_STRING_LEN]; + char mapkey[LONG_STRING_LEN]; + char defaultvalue[LONG_STRING_LEN]; + int n; + + cpI = uri; + cpIE = cpI+strlen(cpI); + cpO = newuri; + while (cpI < cpIE) { + if (cpI+6 < cpIE && strncmp(cpI, "${", 2) == 0) { + /* missing delimiter -> take it as plain text */ + if ( strchr(cpI+2, ':') == NULL + || strchr(cpI+2, '}') == NULL) { + memcpy(cpO, cpI, 2); + cpO += 2; + cpI += 2; + continue; + } + cpI += 2; + + cpT = strchr(cpI, ':'); + n = cpT-cpI; + memcpy(mapname, cpI, limit_length(n)); + mapname[limit_length(n)] = '\0'; + cpI += n+1; + + cpT2 = strchr(cpI, '|'); + cpT = strchr(cpI, '}'); + if (cpT2 != NULL && cpT2 < cpT) { + n = cpT2-cpI; + memcpy(mapkey, cpI, limit_length(n)); + mapkey[limit_length(n)] = '\0'; + cpI += n+1; + + n = cpT-cpI; + memcpy(defaultvalue, cpI, limit_length(n)); + defaultvalue[limit_length(n)] = '\0'; + cpI += n+1; + } + else { + n = cpT-cpI; + memcpy(mapkey, cpI, limit_length(n)); + mapkey[limit_length(n)] = '\0'; + cpI += n+1; + + defaultvalue[0] = '\0'; + } + + cpT = lookup_map(r, mapname, mapkey); + if (cpT != NULL) { + n = strlen(cpT); + if (cpO + n >= newuri + sizeof(newuri)) { + ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, + r, "insufficient space in " + "expand_map_lookups, aborting"); + return; + } + memcpy(cpO, cpT, n); + cpO += n; + } + else { + n = strlen(defaultvalue); + if (cpO + n >= newuri + sizeof(newuri)) { + ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, + r, "insufficient space in " + "expand_map_lookups, aborting"); + return; + } + memcpy(cpO, defaultvalue, n); + cpO += n; + } + } + else { + cpT = strstr(cpI, "${"); + if (cpT == NULL) + cpT = cpI+strlen(cpI); + n = cpT-cpI; + if (cpO + n >= newuri + sizeof(newuri)) { + ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, + r, "insufficient space in " + "expand_map_lookups, aborting"); + return; + } + memcpy(cpO, cpI, n); + cpO += n; + cpI += n; + } + } + *cpO = '\0'; + ap_cpystrn(uri, newuri, uri_len); + return; +} + +#undef limit_length + + + + /* From: Tony Finch To: apbugs@apache.org Cc: Anders Henke Subject: Re: mod_rewrite/6671: backreferences aren't being expanded for internal RewriteMaps Date: Wed, 18 Oct 2000 02:56:12 +0000 Anders Henke wrote: >On Oct 17th, slive@apache.org wrote: >> >> The following is just a quick hack which restores part of the >> pre-1.3.14 behaviour. It works for me, but it has been hardly >> tested, so use it at your own risk. > >The patch works good enough so that our internal rewrite map works >again. :) Thank you for your quick help. Note that it isn't quite adequate because attackers can still insert bogus rewritemap lookups that will be expanded when they should not be. Security through obscurity (keeping the rewrite map name secret) is a feasible work-around for the problem. Tony. -- en oeccget g mtcaa f.a.n.finch v spdlkishrhtewe y dot@dotat.at eatp o v eiti i d. fanf@covalent.net From: Tony Finch To: Anders Henke Cc: apbugs@apache.org Subject: Re: mod_rewrite/6671: backreferences aren't being expanded for internal RewriteMaps Date: Wed, 18 Oct 2000 04:37:44 +0000 OK, here's the patch I have committed. You might like to test it. Tony. -- en oeccget g mtcaa f.a.n.finch v spdlkishrhtewe y dot@dotat.at eatp o v eiti i d. fanf@covalent.net ? diff Index: mod_rewrite.c =================================================================== RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_rewrite.c,v retrieving revision 1.162 retrieving revision 1.163 diff -u -r1.162 -r1.163 --- mod_rewrite.c 2000/09/22 20:47:19 1.162 +++ mod_rewrite.c 2000/10/18 04:26:43 1.163 @@ -2258,30 +2258,50 @@ /* now we have a '$' or a '%' */ if (inp[1] == '{') { char *endp; - endp = strchr(inp, '}'); + endp = find_closing_bracket(inp+2, '{', '}'); if (endp == NULL) { goto skip; } *endp = '\0'; if (inp[0] == '$') { /* ${...} map lookup expansion */ + /* + * To make rewrite maps useful the lookup key and + * default values must be expanded, so we make + * recursive calls to do the work. For security + * reasons we must never expand a string that includes + * verbatim data from the network. The recursion here + * isn't a problem because the result of expansion is + * only passed to lookup_map() so it cannot be + * re-expanded, only re-looked-up. Another way of + * looking at it is that the recursion is entirely + * driven by the syntax of the nested curly brackets. + */ char *key, *dflt, *result; + char xkey[MAX_STRING_LEN]; + char xdflt[MAX_STRING_LEN]; + char *empty = ""; key = strchr(inp, ':'); if (key == NULL) { goto skip; } *key++ = '\0'; dflt = strchr(key, '|'); - if (dflt) { + if (dflt == NULL) { + dflt = empty; + } + else { *dflt++ = '\0'; } - result = lookup_map(r, inp+2, key); + do_expand(r, key, xkey, sizeof(xkey), briRR, briRC); + do_expand(r, dflt, xdflt, sizeof(xdflt), briRR, briRC); + result = lookup_map(r, inp+2, xkey); if (result == NULL) { - result = dflt ? dflt : ""; + result = xdflt; } span = ap_cpystrn(outp, result, space) - outp; key[-1] = ':'; - if (dflt) { + if (dflt != empty) { dflt[-1] = '|'; } } @@ -4141,6 +4161,28 @@ } } return 0; +} + +/* +** +** Find end of bracketed expression +** s points after the opening bracket +** +*/ + +static char *find_closing_bracket(char *s, int left, int right) +{ + int depth; + + for (depth = 1; *s; ++s) { + if (*s == right && --depth == 0) { + return s; + } + else if (*s == left) { + ++depth; + } + } + return NULL; } /*EOF*/ Index: mod_rewrite.h =================================================================== RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_rewrite.h,v retrieving revision 1.72 retrieving revision 1.73 diff -u -r1.72 -r1.73 --- mod_rewrite.h 2000/09/29 17:32:32 1.72 +++ mod_rewrite.h 2000/10/18 04:26:43 1.73 @@ -496,6 +496,9 @@ /* Lexicographic Comparison */ static int compare_lexicography(char *cpNum1, char *cpNum2); + /* Find end of bracketed expression */ +static char *find_closing_bracket(char *s, int left, int right); + #endif /* _MOD_REWRITE_H */ /*EOF*/ State-Changed-From-To: open-closed State-Changed-By: slive State-Changed-When: Thu Oct 26 23:10:21 PDT 2000 State-Changed-Why: The patch to fix this problem is posted to http://httpd.apache.org/dist/apache_1.3.14-fix.diff and will be included in the next version of Apache. >Unformatted: [In order for any reply to be added to the PR database, you need] [to include in the Cc line and make sure the] [subject line starts with the report component and number, with ] [or without any 'Re:' prefixes (such as "general/1098:" or ] ["Re: general/1098:"). If the subject doesn't match this ] [pattern, your message will be misfiled and ignored. The ] ["apbugs" address is not added to the Cc line of messages from ] [the database automatically because of the potential for mail ] [loops. If you do not include this Cc, your reply may be ig- ] [nored unless you are responding to an explicit request from a ] [developer. Reply only with text; DO NOT SEND ATTACHMENTS! ]