SECURITY [CAN-2003-0020]: escape arbitrary data before writing into the

errorlog.

Reviewed by: Mark J Cox

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

diff --git a/CHANGES b/CHANGES
index fbbdc5e42f590fd41d42f004b4f6dcd942808b23..6c9c887b935e42337cf5852ce16bf247115504e1 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@ Changes with Apache 2.1.0-dev
 
   [Remove entries to the current 2.0 section below, when backported]
 
+  *) SECURITY [CAN-2003-0020]: Escape arbitrary data before writing
+     into the errorlog.  [André Malo]
+
   *) mod_expires: Initialize ExpiresDefault to NULL instead of "" to
      avoid reporting an Internal Server error if it is used without
      having been set in the httpd.conf file. PR: 23748, 24459

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index ef5d0aa1215cd7ae916df95a7dea030ec2235fda..1d912aea1a21b0c3c6d7b7e40ce2ebdcbd25ea38 100644 (file)
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -117,6 +117,7 @@
  *                        handler
  * 20030821 (2.1.0-dev) bumped mod_include's entire API
  * 20030821.1 (2.1.0-dev) added XHTML doctypes
+ * 20030821.2 (2.1.0-dev) added ap_escape_errorlog_item
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503230UL /* "AP20" */
@@ -124,7 +125,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20030821
 #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 175ce79de864507bc7762b257534287d9feddfe7..482a3079300b00b49582131133ed85827d750d16 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1381,11 +1381,21 @@ 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
+ * @param str The string to escape
  * @return The escaped string
  */
 AP_DECLARE(char *) ap_escape_logitem(apr_pool_t *p, const char *str);
 
+/**
+ * Escape a string for logging into the error log (without a pool)
+ * @param dest The buffer to write to
+ * @param source The string to escape
+ * @param maxlen The buffer size for the escaped string (including \0)
+ * @return The len of the escaped string (always < maxlen)
+ */
+AP_DECLARE(apr_size_t) ap_escape_errorlog_item(char *dest, const char *source,
+                                               apr_size_t buflen);
+
 /**
  * Construct a full hostname
  * @param p The pool to allocate from

diff --git a/server/log.c b/server/log.c
index d23753ee93d09d5181217b6f00b491d73b1ef5b4..e01d10f45d0f27b481ccfaa990d5565cb646d494 100644 (file)
--- a/server/log.c
+++ b/server/log.c
@@ -401,7 +401,7 @@ static void log_error_core(const char *file, int line, int level,
                            const request_rec *r, apr_pool_t *pool,
                            const char *fmt, va_list args)
 {
-    char errstr[MAX_STRING_LEN];
+    char errstr[MAX_STRING_LEN], scratch[MAX_STRING_LEN];
     apr_size_t len, errstrlen;
     apr_file_t *logf = NULL;
     const char *referer;
@@ -536,12 +536,17 @@ static void log_error_core(const char *file, int line, int level,
             errstr[len] = '\0';
         }
     }
+
     errstrlen = len;
-    len += apr_vsnprintf(errstr + len, MAX_STRING_LEN - len, fmt, args);
+    if (apr_vsnprintf(scratch, MAX_STRING_LEN - len, fmt, args)) {
+        len += ap_escape_errorlog_item(errstr + len, scratch,
+                                       MAX_STRING_LEN - len);
+    }
 
-    if (r && (referer = apr_table_get(r->headers_in, "Referer"))) {
+    if (   r && (referer = apr_table_get(r->headers_in, "Referer"))
+        && ap_escape_errorlog_item(scratch, referer, MAX_STRING_LEN - len)) {
         len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
-                            ", referer: %s", referer);
+                            ", referer: %s", scratch);
     }
 
     /* NULL if we are logging to syslog */

diff --git a/server/util.c b/server/util.c
index b83a8ca59eab0f92e645fd5c98eaab54ac10f06f..8688bcf066dfd81c800da283e44ddd040d43d73c 100644 (file)
--- a/server/util.c
+++ b/server/util.c
@@ -1838,6 +1838,70 @@ AP_DECLARE(char *) ap_escape_logitem(apr_pool_t *p, const char *str)
     return ret;
 }
 
+AP_DECLARE(apr_size_t) ap_escape_errorlog_item(char *dest, const char *source,
+                                               apr_size_t buflen)
+{
+    unsigned char *d, *ep;
+    const unsigned char *s;
+
+    if (!source || !buflen) { /* be safe */
+        return 0;
+    }
+
+    d = (unsigned char *)dest;
+    s = (const unsigned char *)source;
+    ep = d + buflen - 1;
+
+    for (; d < ep && *s; ++s) {
+
+        if (TEST_CHAR(*s, T_ESCAPE_LOGITEM)) {
+            *d++ = '\\';
+            if (d >= ep) {
+                --d;
+                break;
+            }
+
+            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 '\\':
+                *d++ = *s;
+                break;
+            case '"': /* no need for this in error log */
+                d[-1] = *s;
+                break;
+            default:
+                if (d >= ep - 2) {
+                    ep = --d; /* break the for loop as well */
+                    break;
+                }
+                c2x(*s, d);
+                *d = 'x';
+                d += 3;
+            }
+        }
+        else {
+            *d++ = *s;
+        }
+    }
+    *d = '\0';
+
+    return (d - (unsigned char *)dest);
+}
+
 AP_DECLARE(int) ap_is_directory(apr_pool_t *p, const char *path)
 {
     apr_finfo_t finfo;