Date: Wed, 18 Oct 2000 16:27:49 +0000 From: Tony Finch <dot@DOTAT.AT> Subject: Re: Security vulnerability in Apache mod_rewrite To: BUGTRAQ@SECURITYFOCUS.COM Since the Apache Group released 1.3.14 (which incorporates the mod_rewrite security "fix") we have been informed that it breaks most configurations that use RewriteMaps, because the lookup key is no longer expanded. See <http://bugs.apache.org/index.cgi/full/6671>, which includes a patch to restore the lost functionality. I have also included the patch below. Redistributors of Apache should update their packages to incorporate 1.3.14 with this patch. 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*/