Bugtraq mailing list archives

Re: Security vulnerability in Apache mod_rewrite


From: Tony Finch <dot () DOTAT AT>
Date: Fri, 29 Sep 2000 17:37:07 +0000

Kevin van der Raad <k.van.der.raad () ITSEC NL> wrote:

The patch is currently being tested and will be part of the release
of Apache 1.3.13. Until then, users should check their
configuration files and not use rules that map to a filename such
as the first example above.

You can alternatively use this patch, which has been committed.

Tony.
--
en oeccget g mtcaa    f.a.n.finch
v spdlkishrhtewe y    dot () dotat at
eatp o v eiti i d.    fanf () covalent net


Index: mod_rewrite.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_rewrite.c,v
retrieving revision 1.158
retrieving revision 1.160
diff -u -r1.158 -r1.160
--- mod_rewrite.c       2000/07/14 20:01:55     1.158
+++ mod_rewrite.c       2000/09/22 20:34:36     1.160
@@ -1745,7 +1745,6 @@
     char *output;
     const char *vary;
     char newuri[MAX_STRING_LEN];
-    char env[MAX_STRING_LEN];
     regex_t *regexp;
     regmatch_t regmatch[MAX_NMATCH];
     backrefinfo *briRR = NULL;
@@ -1913,20 +1912,7 @@
      *  (`RewriteRule <pat> - [E=...]')
      */
     if (strcmp(output, "-") == 0) {
-        for (i = 0; p->env[i] != NULL; i++) {
-            /*  1. take the string  */
-            ap_cpystrn(env, p->env[i], sizeof(env));
-            /*  2. expand $N (i.e. backrefs to RewriteRule pattern)  */
-            expand_backref_inbuffer(r->pool, env, sizeof(env), briRR, '$');
-            /*  3. expand %N (i.e. backrefs to latest RewriteCond pattern)  */
-            expand_backref_inbuffer(r->pool, env, sizeof(env), briRC, '%');
-            /*  4. expand %{...} (i.e. variables) */
-            expand_variables_inbuffer(r, env, sizeof(env));
-            /*  5. expand ${...} (RewriteMap lookups)  */
-            expand_map_lookups(r, env, sizeof(env));
-            /*  and add the variable to Apache's structures  */
-            add_env_variable(r, env);
-        }
+       do_expand_env(r, p->env, briRR, briRC);
         if (p->forced_mimetype != NULL) {
             if (perdir == NULL) {
                 /* In the per-server context we can force the MIME-type
@@ -1961,17 +1947,7 @@
      *  that there is something to replace, so we create the
      *  substitution URL string in `newuri'.
      */
-    /*  1. take the output string  */
-    ap_cpystrn(newuri, output, sizeof(newuri));
-    /*  2. expand $N (i.e. backrefs to RewriteRule pattern)  */
-    expand_backref_inbuffer(r->pool, newuri, sizeof(newuri), briRR, '$');
-    /*  3. expand %N (i.e. backrefs to latest RewriteCond pattern)  */
-    expand_backref_inbuffer(r->pool, newuri, sizeof(newuri), briRC, '%');
-    /*  4. expand %{...} (i.e. variables) */
-    expand_variables_inbuffer(r, newuri, sizeof(newuri));
-    /*  5. expand ${...} (RewriteMap lookups)  */
-    expand_map_lookups(r, newuri, sizeof(newuri));
-    /*  and log the result... */
+    do_expand(r, output, newuri, sizeof(newuri), briRR, briRC);
     if (perdir == NULL) {
         rewritelog(r, 2, "rewrite %s -> %s", uri, newuri);
     }
@@ -1983,20 +1959,7 @@
      *  Additionally do expansion for the environment variable
      *  strings (`RewriteRule .. .. [E=<string>]').
      */
-    for (i = 0; p->env[i] != NULL; i++) {
-        /*  1. take the string  */
-        ap_cpystrn(env, p->env[i], sizeof(env));
-        /*  2. expand $N (i.e. backrefs to RewriteRule pattern)  */
-        expand_backref_inbuffer(r->pool, env, sizeof(env), briRR, '$');
-        /*  3. expand %N (i.e. backrefs to latest RewriteCond pattern)  */
-        expand_backref_inbuffer(r->pool, env, sizeof(env), briRC, '%');
-        /*  4. expand %{...} (i.e. variables) */
-        expand_variables_inbuffer(r, env, sizeof(env));
-        /*  5. expand ${...} (RewriteMap lookups)  */
-        expand_map_lookups(r, env, sizeof(env));
-        /*  and add the variable to Apache's structures  */
-        add_env_variable(r, env);
-    }
+    do_expand_env(r, p->env, briRR, briRC);

     /*
      *  Now replace API's knowledge of the current URI:
@@ -2163,16 +2126,7 @@
      *   Construct the string we match against
      */

-    /*  1. take the string  */
-    ap_cpystrn(input, p->input, sizeof(input));
-    /*  2. expand $N (i.e. backrefs to RewriteRule pattern)  */
-    expand_backref_inbuffer(r->pool, input, sizeof(input), briRR, '$');
-    /*  3. expand %N (i.e. backrefs to latest RewriteCond pattern)  */
-    expand_backref_inbuffer(r->pool, input, sizeof(input), briRC, '%');
-    /*  4. expand %{...} (i.e. variables) */
-    expand_variables_inbuffer(r, input, sizeof(input));
-    /*  5. expand ${...} (RewriteMap lookups)  */
-    expand_map_lookups(r, input, sizeof(input));
+    do_expand(r, p->input, input, sizeof(input), briRR, briRC);

     /*
      *   Apply the patterns
@@ -2314,8 +2268,141 @@
 ** +-------------------------------------------------------+
 */

+
+/*
+**
+**  perform all the expansions on the input string
+**  leaving the result in the supplied buffer
+**
+*/
+
+static void do_expand(request_rec *r, char *input, char *buffer, int nbuf,
+                      backrefinfo *briRR, backrefinfo *briRC)
+{
+    char *inp, *outp;
+    size_t span, space;
+
+    /*
+     * for security reasons this expansion must be perfomed in a
+     * single pass, otherwise an attacker can arrange for the result
+     * of an earlier expansion to include expansion specifiers that
+     * are interpreted by a later expansion, producing results that
+     * were not intended by the administrator.
+     */
+
+    inp = input;
+    outp = buffer;
+    space = nbuf - 1; /* room for '\0' */
+
+    for (;;) {
+       span = strcspn(inp, "$%");
+       if (span > space) {
+           span = space;
+       }
+       memcpy(outp, inp, span);
+       inp += span;
+       outp += span;
+       space -= span;
+       if (space == 0 || *inp == '\0') {
+           break;
+       }
+       /* now we have a '$' or a '%' */
+       if (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;
+           }
+           *endp = '}';
+           inp = endp+1;
+           outp += span;
+           space -= span;
+           continue;
+       }
+       else if (ap_isdigit(inp[1])) {
+           int n = inp[1] - '0';
+           backrefinfo *bri = NULL;
+           if (inp[0] == '$') {
+               /* $N RewriteRule regexp backref expansion */
+               bri = briRR;
+           }
+           else if (inp[0] == '%') {
+               /* %N RewriteCond regexp backref expansion */
+               bri = briRC;
+           }
+           /* see ap_pregsub() in src/main/util.c */
+            if (bri && n <= bri->nsub &&
+               bri->regmatch[n].rm_eo > bri->regmatch[n].rm_so) {
+               span = bri->regmatch[n].rm_eo - bri->regmatch[n].rm_so;
+               if (span > space) {
+                   span = space;
+               }
+               memcpy(outp, bri->source + bri->regmatch[n].rm_so, span);
+               outp += span;
+               space -= span;
+           }
+           inp += 2;
+           continue;
+       }
+    skip:
+       *outp++ = *inp++;
+       space--;
+    }
+    *outp++ = '\0';
+}
+
+
 /*
 **
+**  perform all the expansions on the environment variables
+**
+*/
+
+static void do_expand_env(request_rec *r, char *env[],
+                         backrefinfo *briRR, backrefinfo *briRC)
+{
+    int i;
+    char buf[MAX_STRING_LEN];
+
+    for (i = 0; env[i] != NULL; i++) {
+       do_expand(r, env[i], buf, sizeof(buf), briRR, briRC);
+       add_env_variable(r, buf);
+    }
+}
+
+
+/*
+**
 **  split out a QUERY_STRING part from
 **  the current URI string
 **
@@ -2484,51 +2571,6 @@

 /*
 **
-**  Expand the %0-%9 or $0-$9 regex backreferences
-**
-*/
-
-static void expand_backref_inbuffer(pool *p, char *buf, int nbuf,
-                                    backrefinfo *bri, char c)
-{
-    register int i;
-
-    /* protect existing $N and & backrefs and replace <c>N with $N backrefs */
-    for (i = 0; buf[i] != '\0' && i < nbuf; i++) {
-        if (buf[i] == '\\' && (buf[i+1] != '\0' && i < (nbuf-1))) {
-            i++; /* protect next */
-        }
-        else if (buf[i] == '&') {
-            buf[i] = '\001';
-        }
-        else if (c != '$' && buf[i] == '$' && (buf[i+1] >= '0' && buf[i+1] <= '9')) {
-            buf[i] = '\002';
-            i++; /* speedup */
-        }
-        else if (buf[i] == c && (buf[i+1] >= '0' && buf[i+1] <= '9')) {
-            buf[i] = '$';
-            i++; /* speedup */
-        }
-    }
-
-    /* now apply the standard regex substitution function */
-    ap_cpystrn(buf, ap_pregsub(p, buf, bri->source,
-                               bri->nsub+1, bri->regmatch), nbuf);
-
-    /* restore the original $N and & backrefs */
-    for (i = 0; buf[i] != '\0' && i < nbuf; i++) {
-        if (buf[i] == '\001') {
-            buf[i] = '&';
-        }
-        else if (buf[i] == '\002') {
-            buf[i] = '$';
-        }
-    }
-}
-
-
-/*
-**
 **  Expand tilde-paths (/~user) through
 **  Unix /etc/passwd database information
 **
@@ -2571,121 +2613,8 @@
 }
 #endif

-/*
-**
-**  mapfile expansion support
-**  i.e. expansion of MAP lookup directives
-**  ${<mapname>:<key>} 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
-
-
-
 /*
 ** +-------------------------------------------------------+
 ** |                                                       |
@@ -3478,53 +3407,6 @@
 ** +-------------------------------------------------------+
 */

-
-static void expand_variables_inbuffer(request_rec *r, char *buf, int buf_len)
-{
-    char *newbuf;
-    newbuf = expand_variables(r, buf);
-    if (strcmp(newbuf, buf) != 0) {
-        ap_cpystrn(buf, newbuf, buf_len);
-    }
-    return;
-}
-
-static char *expand_variables(request_rec *r, char *str)
-{
-    char output[MAX_STRING_LEN];
-    char input[MAX_STRING_LEN];
-    char *cp;
-    char *cp2;
-    char *cp3;
-    int expanded;
-    char *outp;
-    char *endp;
-
-    ap_cpystrn(input, str, sizeof(input));
-    output[0] = '\0';
-    outp = output;
-    endp = output + sizeof(output);
-    expanded = 0;
-    for (cp = input; cp < input+MAX_STRING_LEN; ) {
-        if ((cp2 = strstr(cp, "%{")) != NULL) {
-            if ((cp3 = strstr(cp2, "}")) != NULL) {
-                *cp2 = '\0';
-                outp = ap_cpystrn(outp, cp, endp - outp);
-
-                cp2 += 2;
-                *cp3 = '\0';
-                outp = ap_cpystrn(outp, lookup_variable(r, cp2), endp - outp);
-
-                cp = cp3+1;
-                expanded = 1;
-                continue;
-            }
-        }
-        outp = ap_cpystrn(outp, cp, endp - outp);
-        break;
-    }
-    return expanded ? ap_pstrdup(r->pool, output) : str;
-}

 static char *lookup_variable(request_rec *r, char *var)
 {
Index: mod_rewrite.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/modules/standard/mod_rewrite.h,v
retrieving revision 1.67
retrieving revision 1.69
diff -u -r1.67 -r1.69
--- mod_rewrite.h       2000/09/14 11:54:14     1.67
+++ mod_rewrite.h       2000/09/22 20:34:36     1.69
@@ -420,14 +420,16 @@
                               char *perdir, backrefinfo *briRR,
                               backrefinfo *briRC);

+static void do_expand(request_rec *r, char *input, char *buffer, int nbuf,
+                      backrefinfo *briRR, backrefinfo *briRC);
+static void do_expand_env(request_rec *r, char *env[],
+                         backrefinfo *briRR, backrefinfo *briRC);
+
     /* URI transformation function */
 static void  splitout_queryargs(request_rec *r, int qsappend);
 static void  fully_qualify_uri(request_rec *r);
 static void  reduce_uri(request_rec *r);
-static void  expand_backref_inbuffer(pool *p, char *buf, int nbuf,
-                                     backrefinfo *bri, char c);
 static char *expand_tildepaths(request_rec *r, char *uri);
-static void  expand_map_lookups(request_rec *r, char *uri, int uri_len);

     /* rewrite map support functions */
 static char *lookup_map(request_rec *r, char *name, char *key);
@@ -466,8 +468,6 @@
 static int   rewritemap_program_child(void *cmd, child_info *pinfo);

     /* env variable support */
-static void  expand_variables_inbuffer(request_rec *r, char *buf, int buf_len);
-static char *expand_variables(request_rec *r, char *str);
 static char *lookup_variable(request_rec *r, char *var);
 static char *lookup_header(request_rec *r, const char *name);


Current thread: