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 bug got "ported" from Apache 1.3: see bugs 16661, 11998, 8906, 8048, 5811. Here is a patch that has been thoroughly tested (more details at http://www.manniwood.net/mod_usertrack_patch.html): --- mod_usertrack.c 2002-07-17 12:08:53.000000000 -0400 +++ mod_usertrack_2.0_fixed.c 2003-01-31 13:12:59.000000000 -0500 @@ -125,6 +125,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; /* Make Cookie: Now we have to generate something that is going to be @@ -197,31 +199,44 @@ { cookie_dir_rec *dcfg = ap_get_module_config(r->per_dir_config, &usertrack_module); - const char *cookie; - const 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 = apr_table_get(r->headers_in, - (dcfg->style == CT_COOKIE2 - ? "Cookie2" - : "Cookie")))) - if ((value = ap_strstr_c(cookie, dcfg->cookie_name))) { - char *cookiebuf, *cookieend; - - value += strlen(dcfg->cookie_name) + 1; /* Skip over the '=' */ - cookiebuf = apr_pstrdup(r->pool, value); - cookieend = strchr(cookiebuf, ';'); - if (cookieend) - *cookieend = '\0'; /* Ignore anything after a ; */ - - /* Set the cookie in a note, for logging */ - apr_table_setn(r->notes, "cookie", cookiebuf); + if ((cookie_header = apr_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 */ + apr_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 */ } @@ -330,7 +345,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 = apr_pstrcat(cmd->pool, "^", name, "=([^;]+)|;[ \t]+", name, "=([^;]+)", NULL); + dcfg->cookie_name = apr_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 4675 [details] patch for spot_cookie() bug in mod_usertrack.c
*** This bug has been marked as a duplicate of 16661 ***