Minor MMN bump:

Forward port: Escape special characters (especially control
characters) in mod_log_config to make a clear distinction between
client-supplied strings (with special characters) and server-side
strings. This was already introduced in version 1.3.25.

Obtained from: Patch in 1.3.25-dev by Martin


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

6 files changed, 114 insertions, 26 deletions

diff --git a/CHANGES b/CHANGES
index ae1ed2fb7a..f3f202d67a 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,12 @@ Changes with Apache 2.1.0-dev
 
   [Remove entries to the current 2.0 section below, when backported]
 
+  *) Forward port: Escape special characters (especially control
+     characters) in mod_log_config to make a clear distinction between
+     client-supplied strings (with special characters) and server-side
+     strings. This was already introduced in version 1.3.25.
+     [André Malo]
+
   *) mod_usertrack: don't set the cookie in subrequests. This works
      around the problem that cookies were set twice during fast internal
      redirects. PR 13211.  [André Malo]
diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 84744b8035..9e029cfc30 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -112,7 +112,9 @@
  * 20020628 (2.0.40-dev) Added filter_init to filter registration functions
  * 20020903 (2.0.41-dev) APR's error constants changed
  * 20020903.1 (2.1.0-dev) allow_encoded_slashes added to core_dir_config
- * 20030213.1 (2.1.0-dev) changed log_writer optional fn's to return previous handler
+ * 20030213.1 (2.1.0-dev) changed log_writer optional fn's to return previous
+ *                        handler
+ * 20030213.2 (2.1.0-dev) add ap_escape_logitem
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */
@@ -120,7 +122,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20030213
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 1                     /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 2                     /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
diff --git a/include/httpd.h b/include/httpd.h
index b922506ca4..cfbbab6c13 100644
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1360,6 +1360,14 @@ AP_DECLARE(char *) ap_os_escape_path(apr_pool_t *p, const char *path, int partia
 AP_DECLARE(char *) ap_escape_html(apr_pool_t *p, const char *s);
 
 /**
+ * Escape a string for logging
+ * @param p The pool to allocate from
+ * @param s The string to escape
+ * @return The escaped string
+ */
+AP_DECLARE(char *) ap_escape_logitem(apr_pool_t *p, const char *str);
+
+/**
  * Construct a full hostname
  * @param p The pool to allocate from
  * @param hostname The hostname of the server
diff --git a/modules/loggers/mod_log_config.c b/modules/loggers/mod_log_config.c
index 27f3e5f9d5..032b806d9b 100644
--- a/modules/loggers/mod_log_config.c
+++ b/modules/loggers/mod_log_config.c
@@ -333,8 +333,9 @@ static const char *constant_item(request_rec *dummy, char *stuff)
 
 static const char *log_remote_host(request_rec *r, char *a)
 {
-    return ap_get_remote_host(r->connection, r->per_dir_config,
-                                    REMOTE_NAME, NULL);
+    return ap_escape_logitem(r->pool, ap_get_remote_host(r->connection,
+                                                         r->per_dir_config,
+                                                         REMOTE_NAME, NULL));
 }
 
 static const char *log_remote_address(request_rec *r, char *a)
@@ -349,7 +350,7 @@ static const char *log_local_address(request_rec *r, char *a)
 
 static const char *log_remote_logname(request_rec *r, char *a)
 {
-    return ap_get_remote_logname(r);
+    return ap_escape_logitem(r->pool, ap_get_remote_logname(r));
 }
 
 static const char *log_remote_user(request_rec *r, char *a)
@@ -362,6 +363,10 @@ static const char *log_remote_user(request_rec *r, char *a)
     else if (strlen(rvalue) == 0) {
         rvalue = "\"\"";
     }
+    else {
+        rvalue = ap_escape_logitem(r->pool, rvalue);
+    }
+
     return rvalue;
 }
 
@@ -372,33 +377,37 @@ static const char *log_request_line(request_rec *r, char *a)
      * (note the truncation before the protocol string for HTTP/0.9 requests)
      * (note also that r->the_request contains the unmodified request)
      */
-    return (r->parsed_uri.password) 
-                ? apr_pstrcat(r->pool, r->method, " ",
-                              apr_uri_unparse(r->pool, &r->parsed_uri, 0),
-                              r->assbackwards ? NULL : " ", r->protocol, NULL)
-                : r->the_request;
+    return ap_escape_logitem(r->pool,
+                             (r->parsed_uri.password)
+                               ? apr_pstrcat(r->pool, r->method, " ",
+                                             apr_uri_unparse(r->pool,
+                                                             &r->parsed_uri, 0),
+                                             r->assbackwards ? NULL : " ",
+                                             r->protocol, NULL)
+                               : r->the_request);
 }
 
 static const char *log_request_file(request_rec *r, char *a)
 {
-    return r->filename;
+    return ap_escape_logitem(r->pool, r->filename);
 }
 static const char *log_request_uri(request_rec *r, char *a)
 {
-    return r->uri;
+    return ap_escape_logitem(r->pool, r->uri);
 }
 static const char *log_request_method(request_rec *r, char *a)
 {
-    return r->method;
+    return ap_escape_logitem(r->pool, r->method);
 }
 static const char *log_request_protocol(request_rec *r, char *a)
 {
-    return r->protocol;
+    return ap_escape_logitem(r->pool, r->protocol);
 }
 static const char *log_request_query(request_rec *r, char *a)
 {
-    return (r->args != NULL) ? apr_pstrcat(r->pool, "?", r->args, NULL)
-                             : "";
+    return (r->args) ? apr_pstrcat(r->pool, "?",
+                                   ap_escape_logitem(r->pool, r->args), NULL)
+                     : "";
 }
 static const char *log_status(request_rec *r, char *a)
 {
@@ -428,7 +437,7 @@ static const char *log_bytes_sent(request_rec *r, char *a)
 
 static const char *log_header_in(request_rec *r, char *a)
 {
-    return apr_table_get(r->headers_in, a);
+    return ap_escape_logitem(r->pool, apr_table_get(r->headers_in, a));
 }
 
 static const char *log_header_out(request_rec *r, char *a)
@@ -438,18 +447,18 @@ static const char *log_header_out(request_rec *r, char *a)
         cp = ap_field_noparam(r->pool, r->content_type);
     }
     if (cp) {
-        return cp;
+        return ap_escape_logitem(r->pool, cp);
     }
-    return apr_table_get(r->err_headers_out, a);
+    return ap_escape_logitem(r->pool, apr_table_get(r->err_headers_out, a));
 }
 
 static const char *log_note(request_rec *r, char *a)
 {
-    return apr_table_get(r->notes, a);
+    return ap_escape_logitem(r->pool, apr_table_get(r->notes, a));
 }
 static const char *log_env_var(request_rec *r, char *a)
 {
-    return apr_table_get(r->subprocess_env, a);
+    return ap_escape_logitem(r->pool, apr_table_get(r->subprocess_env, a));
 }
 
 static const char *log_cookie(request_rec *r, char *a)
@@ -467,7 +476,7 @@ static const char *log_cookie(request_rec *r, char *a)
             if (end_cookie) {
                 *end_cookie = '\0';
             }
-            return cookie;
+            return ap_escape_logitem(r->pool, cookie);
         }
     }
     return NULL;
@@ -585,7 +594,7 @@ static const char *log_request_duration_microseconds(request_rec *r, char *a)
  */
 static const char *log_virtual_host(request_rec *r, char *a)
 {
-    return r->server->server_hostname;
+    return ap_escape_logitem(r->pool, r->server->server_hostname);
 }
 
 static const char *log_server_port(request_rec *r, char *a)
@@ -599,7 +608,7 @@ static const char *log_server_port(request_rec *r, char *a)
  */
 static const char *log_server_name(request_rec *r, char *a)
 {
-    return ap_get_server_name(r);
+    return ap_escape_logitem(r->pool, ap_get_server_name(r));
 }
 
 static const char *log_child_pid(request_rec *r, char *a)
diff --git a/server/gen_test_char.c b/server/gen_test_char.c
index faab8126fb..5102ca98c0 100644
--- a/server/gen_test_char.c
+++ b/server/gen_test_char.c
@@ -73,6 +73,7 @@
 #define T_ESCAPE_PATH_SEGMENT (0x02)
 #define T_OS_ESCAPE_PATH      (0x04)
 #define T_HTTP_TOKEN_STOP     (0x08)
+#define T_ESCAPE_LOGITEM      (0x10)
 
 int main(int argc, char *argv[])
 {
@@ -85,13 +86,15 @@ int main(int argc, char *argv[])
            "#define T_ESCAPE_PATH_SEGMENT  (%u)\n"
            "#define T_OS_ESCAPE_PATH       (%u)\n"
            "#define T_HTTP_TOKEN_STOP      (%u)\n"
+           "#define T_ESCAPE_LOGITEM       (%u)\n"
            "\n"
            "static const unsigned char test_char_table[256] = {\n"
            "    0,",
            T_ESCAPE_SHELL_CMD,
            T_ESCAPE_PATH_SEGMENT,
            T_OS_ESCAPE_PATH,
-           T_HTTP_TOKEN_STOP);
+           T_HTTP_TOKEN_STOP,
+           T_ESCAPE_LOGITEM);
 
     /* we explicitly dealt with NUL above
      * in case some strchr() do bogosity with it */
@@ -135,8 +138,16 @@ int main(int argc, char *argv[])
             flags |= T_HTTP_TOKEN_STOP;
         }
 
-        printf("%u%c", flags, (c < 255) ? ',' : ' ');
+        /* For logging, escape all control characters,
+         * double quotes (because they delimit the request in the log file)
+         * backslashes (because we use backslash for escaping)
+         * and 8-bit chars with the high bit set
+         */
+        if (!apr_isprint(c) || c == '"' || c == '\\' || apr_iscntrl(c)) {
+            flags |= T_ESCAPE_LOGITEM;
+        }
 
+        printf("%u%c", flags, (c < 255) ? ',' : ' ');
     }
 
     printf("\n};\n");
diff --git a/server/util.c b/server/util.c
index 246274edf5..3e6c55bc29 100644
--- a/server/util.c
+++ b/server/util.c
@@ -1784,6 +1784,58 @@ AP_DECLARE(char *) ap_escape_html(apr_pool_t *p, const char *s)
     return x;
 }
 
+AP_DECLARE(char *) ap_escape_logitem(apr_pool_t *p, const char *str)
+{
+    char *ret;
+    unsigned char *d;
+    const unsigned char *s;
+
+    if (!str) {
+        return NULL;
+    }
+
+    ret = apr_palloc(p, 4 * strlen(str) + 1); /* Be safe */
+    d = (unsigned char *)ret;
+    s = (const unsigned char *)str;
+    for (; *s; ++s) {
+
+        if (TEST_CHAR(*s, T_ESCAPE_LOGITEM)) {
+            *d++ = '\\';
+            switch(*s) {
+            case '\b':
+                *d++ = 'b';
+                break;
+            case '\n':
+                *d++ = 'n';
+                break;
+            case '\r':
+                *d++ = 'r';
+                break;
+            case '\t':
+                *d++ = 't';
+                break;
+            case '\v':
+                *d++ = 'v';
+                break;
+            case '\\':
+            case '"':
+                *d++ = *s;
+                break;
+            default:
+                c2x(*s, d);
+                *d = 'x';
+                d += 3;
+            }
+        }
+        else {
+            *d++ = *s;
+        }
+    }
+    *d = '\0';
+
+    return ret;
+}
+
 AP_DECLARE(int) ap_is_directory(apr_pool_t *p, const char *path)
 {
     apr_finfo_t finfo;