You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jw...@apache.org on 2003/09/24 00:40:23 UTC
cvs commit: httpd-2.0/modules/metadata mod_usertrack.c
jwoolley 2003/09/23 15:40:23
Modified: . CHANGES
modules/metadata mod_usertrack.c
Log:
The problem that this patch solves is one where cookie names are mis-identified
by mod_usertrack. This is because of the use of strstr() in spot_cookie() the
original mod_usertrack.c to find the name of the cookie. strstr(), by virtue of
looking for a substring instead of an exact match, can mis-identify the cookie
"MyID" as the cookie "ID" or "My". So, if you were looking for the value of the
cookie "ID", but only the cookie "MyID" was returned by the browser,
mod_usertrack.c would return the value of the "MyID" cookie in place of the
"ID" you were looking for.
Even more seriously, because strstr is invoked before the cookie name is
separated from its cookie value, a cookie and value like
"myCookie=thisisnotIDeal" will be a false positive if you told mod_usertrack
the cookie name was ID. Furthermore, using this example, "eal" will get logged
as the value of the cookie; now that strstr has incorrectly identified the
substring "ID" as the cookie name, the following "e" (assumed to be the "="
sign) gets discarded, and the remaining content used as the value of
the cookie.
Replacing the strstr() with a more robust regex match fixes this problem.
PR: 16661
Submitted by: Manni Wood <ma...@planet-save.com>
Revision Changes Path
1.1278 +4 -0 httpd-2.0/CHANGES
Index: CHANGES
===================================================================
RCS file: /home/cvs/httpd-2.0/CHANGES,v
retrieving revision 1.1277
retrieving revision 1.1278
diff -u -d -u -r1.1277 -r1.1278
--- CHANGES 19 Sep 2003 20:02:04 -0000 1.1277
+++ CHANGES 23 Sep 2003 22:40:23 -0000 1.1278
@@ -2,6 +2,10 @@
[Remove entries to the current 2.0 section below, when backported]
+ *) Fixed mod_usertrack to not get false positive matches on the
+ user-tracking cookie's name. PR 16661.
+ [Manni Wood <ma...@planet-save.com>]
+
*) Fix mod_info to use the real config file name, not the default
config file name. [Aryeh Katz <ar...@secured-services.com>]
1.42 +49 -16 httpd-2.0/modules/metadata/mod_usertrack.c
Index: mod_usertrack.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/metadata/mod_usertrack.c,v
retrieving revision 1.41
retrieving revision 1.42
diff -u -d -u -r1.41 -r1.42
--- mod_usertrack.c 6 Mar 2003 23:12:29 -0000 1.41
+++ mod_usertrack.c 23 Sep 2003 22:40:23 -0000 1.42
@@ -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
@@ -193,36 +195,48 @@
return;
}
+/* dcfg->regexp is "^cookie_name=([^;]+)|;[ \t]+cookie_name=([^;]+)",
+ * which has three subexpressions, $0..$2 */
+#define NUM_SUBS 3
+
static int spot_cookie(request_rec *r)
{
cookie_dir_rec *dcfg = ap_get_module_config(r->per_dir_config,
&usertrack_module);
- const char *cookie;
- const char *value;
+ const char *cookie_header;
+ regmatch_t regm[NUM_SUBS];
/* Do not run in subrequests */
if (!dcfg->enabled || r->main) {
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 ; */
-
+ if ((cookie_header = apr_table_get(r->headers_in,
+ (dcfg->style == CT_COOKIE2
+ ? "Cookie2"
+ : "Cookie")))) {
+ if (!ap_regexec(dcfg->regexp, cookie_header, NUM_SUBS, 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,
+ NUM_SUBS, regm);
+ }
+ if (regm[2].rm_so != -1) {
+ cookieval = ap_pregsub(r->pool, "$2", cookie_header,
+ NUM_SUBS, regm);
+ }
/* Set the cookie in a note, for logging */
- apr_table_setn(r->notes, "cookie", cookiebuf);
+ apr_table_setn(r->notes, "cookie", cookieval);
return DECLINED; /* There's already a cookie, no new one */
}
+ }
make_cookie(r);
return OK; /* We set our cookie */
}
@@ -331,7 +345,26 @@
{
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.";
+ }
+ if (dcfg->regexp->re_nsub + 1 != NUM_SUBS) {
+ return apr_pstrcat(cmd->pool, "Invalid cookie name \"",
+ name, "\"", NULL);
+ }
+
return NULL;
}