Hi. I am submitting a patch to mod_usertrack for both Apache 2.0 and 1.3 for your consideration.
The patch fixes a bug where the use of strstr() to find the name of the cookie in the cookieheader can accidentally "find" the name of the cookie in what is actually the contents of a cookie if the contents happen to contain the name of the user tracking cookie. The patch relies on a robust regexp to find the cookiename in the header instead of strstr(). More details are at http://www.manniwood.net/mod_usertrack_patch.html. Patches for 2.0 and 1.3 below. -Manni -------------------------------------------- 2.0: -------------------------------------------- --- 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; } -------------------------------------------- 1.3: -------------------------------------------- --- 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; } -------------------------------------------- Manni Wood, Programmer, Digitas 800 Boylston Street, Boston, MA, 02199 617 867 1881 [EMAIL PROTECTED] "Most men would rather die than think. Many do." --Bertrand Russell
