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 + ? "Cookie2" + : "Cookie")))) { + 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; }
Created attachment 4674 [details] patch for spot_cookie() bug in mod_usertrack
*** Bug 11998 has been marked as a duplicate of this bug. ***
*** Bug 16662 has been marked as a duplicate of this bug. ***
Fixed in 2.1.0-dev, thanks Manni. Backports to 2.0-dev and 1.3-dev will be proposed.