CVE-2014-0098 (reported by Rainer Canavan <rainer-apache 7val com>)

Segfaults w/ truncated cookie logging.

Clean up the cookie logging parser to recognize only the cookie=value pairs,
not valueless cookies.  This refactors multiple passes over the same string
buffer into a single pass parser.

Submitted by: wrowe
Reviewed by: rpluem, jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1575400 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 64ea16d6959ccedffcfddac4ee3f9c0bb7b4fb37..cd69635d119c81838d4336238092bca6242ec04f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) Clean up cookie logging with fewer redundant string parsing passes.
+     Log only cookies with a value assignment.
+     [William Rowe, Ruediger Pluem, Jim Jagielski]
+
   *) mod_ssl: Do not perform SNI / Host header comparison in case of a
      forward proxy request. [Ruediger Pluem]
 

diff --git a/modules/loggers/mod_log_config.c b/modules/loggers/mod_log_config.c
index 5e666ed4089ff5e3ad4eb7a39b9092cff4fb75a9..a34e4c4b4414d049e36a2e0d88e6195444fc622d 100644 (file)
--- a/modules/loggers/mod_log_config.c
+++ b/modules/loggers/mod_log_config.c
@@ -543,14 +543,24 @@ static const char *log_cookie(request_rec *r, char *a)
 
         while ((cookie = apr_strtok(cookies, ";", &last1))) {
             char *name = apr_strtok(cookie, "=", &last2);
-            if (name) {
-                char *value = name + strlen(name) + 1;
-                apr_collapse_spaces(name, name);
+            /* last2 points to the next char following an '=' delim,
+               or the trailing NUL char of the string */
+            char *value = last2;
+            if (name && *name &&  value && *value) {
+                char *last = value - 2;
+                /* Move past leading WS */
+                name += strspn(name, " \t");
+                while (last >= name && apr_isspace(*last)) {
+                    *last = '\0';
+                    --last;
+                }
 
                 if (!strcasecmp(name, a)) {
-                    char *last;
-                    value += strspn(value, " \t");  /* Move past leading WS */
-                    last = value + strlen(value) - 1;
+                    /* last1 points to the next char following the ';' delim,
+                       or the trailing NUL char of the string */
+                    last = last1 - (*last1 ? 2 : 1);
+                    /* Move past leading WS */
+                    value += strspn(value, " \t");
                     while (last >= value && apr_isspace(*last)) {
                        *last = '\0';
                        --last;
@@ -559,6 +569,7 @@ static const char *log_cookie(request_rec *r, char *a)
                     return ap_escape_logitem(r->pool, value);
                 }
             }
+            /* Iterate the remaining tokens using apr_strtok(NULL, ...) */
             cookies = NULL;
         }
     }