DO NOT REPLY [Bug 16661] New: - use of strstr() in spot_cookie() mis-identifies cookies in other cookie names or cookie values

From: <bugzilla(at)apache.org>
Date: Fri Jan 31 2003 - 19:19:30 EST

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16661>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE. http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16661

use of strstr() in spot_cookie() mis-identifies cookies in other cookie names or cookie values

           Summary: use of strstr() in spot_cookie() mis-identifies cookies
                    in other cookie names or cookie values
           Product: Apache httpd-1.3
           Version: 1.3.27
          Platform: Other
               URL: 
http://www.manniwood.net/mod_usertrack_patch.html
        OS/Version: Other
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: Other mods
        AssignedTo: bugs@httpd.apache.org
        ReportedBy: manniwood@planet-save.com

Example: If you have CookieName set to "ID", then use of strstr() in spot_cookie() mod_usertrack.c will get false positives on the following sorts of cookies: "MyID=binky", "MyCookie=IDExpired". This follows up bugs 11998, 8906, 8048, 5811, and probably others. This bug keeps getting submitted. Here is a patch that has been thoroughly tested (more details at http://www.manniwood.net/mod_usertrack_patch.html):

• mod_usertrack.c 2002-03-13 16:05:34.000000000 -0500 +++ mod_usertrack_1.3_fixed.c 2003-01-31 12:10:46.000000000 -0500 @@ -119,6 +119,8 @@ cookie_type_e style; char *cookie_name; char *cookie_domain;
  + char *regexp_string; /* used to compile regexp; save for debugging */
  + regex_t *regexp; /* used to find usertrack cookie in cookie header */
  } cookie_dir_rec;

 /* Define this to allow post-2000 cookies. Cookies use two-digit dates, @@ -250,31 +252,44 @@
 {

     cookie_dir_rec *dcfg = ap_get_module_config(r->per_dir_config,
 						&usertrack_module);

- const char *cookie;
- char *value;
+ const char *cookie_header;

+

+    /* There are only three possibilities from the regexp

+     * ^cookie_name=([^;]+)|;[ \t]+cookie_name=([^;]+)

+     * because $0 is always filled with the whole match, and $1 and $2 will

+     * be filled with either of the parenthesis matches. So, I 

+     * allocate regm[3] to cover all these cases. */

+ regmatch_t regm[3];
+ int i;
 

     if (!dcfg->enabled) {
         return DECLINED;
     }
 
-    if ((cookie = ap_table_get(r->headers_in,
-                               (dcfg->style == CT_COOKIE2
-                                ? "Cookie2"
-                                : "Cookie"))))
-        if ((value = strstr(cookie, dcfg->cookie_name))) {
-            char *cookiebuf, *cookieend;
-
-            value += strlen(dcfg->cookie_name) + 1;  /* Skip over the '=' */
-            cookiebuf = ap_pstrdup(r->pool, value);
-            cookieend = strchr(cookiebuf, ';');
-            if (cookieend)
-                *cookieend = '\0';      /* Ignore anything after a ; */
-
-            /* Set the cookie in a note, for logging */
-            ap_table_setn(r->notes, "cookie", cookiebuf);

+    if ((cookie_header = ap_table_get(r->headers_in,

+                                      (dcfg->style == CT_COOKIE2

+	if (!ap_regexec(dcfg->regexp, cookie_header, dcfg->regexp->re_nsub + 1, regm,
0)) {
+	    char *cookieval = NULL;
+	    /* Our regexp,
+	     * ^cookie_name=([^;]+)|;[ \t]+cookie_name=([^;]+)
+	     * only allows for $1 or $2 to be available. ($0 is always
+	     * filled with the entire matched expression, not just
+	     * the part in parentheses.) So just check for either one
+	     * and assign to cookieval if present. */
+	    if (regm[1].rm_so != -1) {
+		cookieval = ap_pregsub(r->pool, "$1", cookie_header, dcfg->regexp->re_nsub +
1, regm);
+	    }
+	    if (regm[2].rm_so != -1) {
+		cookieval = ap_pregsub(r->pool, "$2", cookie_header, dcfg->regexp->re_nsub +
1, regm);
+	    }
+	    /* Set the cookie in a note, for logging */
+	    ap_table_setn(r->notes, "cookie", cookieval);
 
-            return DECLINED;    /* There's already a cookie, no new one */
-        }
+	    return DECLINED;    /* There's already a cookie, no new one */
+	}

+    }

     make_cookie(r);
     return OK;                  /* We set our cookie */

 }
@@ -382,7 +397,20 @@
 {

     cookie_dir_rec *dcfg = (cookie_dir_rec *) mconfig;  

+ /* The goal is to end up with this regexp,
+ * ^cookie_name=([^;]+)|;[ \t]+cookie_name=([^;]+)
+ * with cookie_name
+ * obviously substituted with the real cookie name set by the
+ * user in httpd.conf. */
+ dcfg->regexp_string = ap_pstrcat(cmd->pool, "^", name, "=([^;]+)|;[ \t]+",
name, "=([^;]+)", NULL);
+

     dcfg->cookie_name = ap_pstrdup(cmd->pool, name);

+

+ dcfg->regexp = ap_pregcomp(cmd->pool, dcfg->regexp_string, REG_EXTENDED);
+ if (dcfg->regexp == NULL) {

+	return "Regular expression could not be compiled.";

+ }

+

     return NULL;
 }

---

To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org For additional commands, e-mail: bugs-help@httpd.apache.org Received on Sat Feb 01 00:17:56 2003