]> granicus.if.org Git - apache/commitdiff
SECURITY (CVE-2014-0117): Fix a crash in mod_proxy. In a reverse
authorJoe Orton <jorton@apache.org>
Tue, 15 Jul 2014 12:27:00 +0000 (12:27 +0000)
committerJoe Orton <jorton@apache.org>
Tue, 15 Jul 2014 12:27:00 +0000 (12:27 +0000)
proxy configuration, a remote attacker could send a carefully crafted
request which could crash a server process, resulting in denial of
service.

Thanks to Marek Kroemeke working with HP's Zero Day Initiative for
reporting this issue.

* server/util.c (ap_parse_token_list_strict): New function.

* modules/proxy/proxy_util.c (find_conn_headers): Use it here.

* modules/proxy/mod_proxy_http.c (ap_proxy_http_process_response):
  Send a 400 for a malformed Connection header.

Submitted by: Edward Lu, breser, covener

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

include/ap_mmn.h
include/httpd.h
modules/proxy/mod_proxy_http.c
modules/proxy/proxy_util.c
server/util.c

index 4c82c364dc8e57e52b2ba3db9e6766f18ec3c06b..ee60c646f1ddf2319a1ed4ab333a421bf01f411f 100644 (file)
  * 20140627.2 (2.5.0-dev)  Added is_name_matchable to proxy_worker_shared.
                            Added ap_proxy_define_match_worker().
  * 20140627.3 (2.5.0-dev)  Add ap_copy_scoreboard_worker()
+ * 20140627.4 (2.5.0-dev)  Added ap_parse_token_list_strict() to httpd.h.
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20140627
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 3                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 4                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index d458ad70ad253ffef8f4a05a0670d3e4710b22d7..77345b2bfc364bff74fb0a2df203a56a07bc9d20 100644 (file)
@@ -1562,6 +1562,23 @@ AP_DECLARE(int) ap_find_etag_weak(apr_pool_t *p, const char *line, const char *t
  */
 AP_DECLARE(int) ap_find_etag_strong(apr_pool_t *p, const char *line, const char *tok);
 
+/**
+ * Retrieve an array of tokens in the format "1#token" defined in RFC2616. Only
+ * accepts ',' as a delimiter, does not accept quoted strings, and errors on
+ * any separator.
+ * @param p The pool to allocate from
+ * @param tok The line to read tokens from
+ * @param tokens Pointer to an array of tokens. If not NULL, must be an array
+ *    of char*, otherwise it will be allocated on @a p when a token is found
+ * @param skip_invalid If true, when an invalid separator is encountered, it
+ *    will be ignored.
+ * @return NULL on success, an error string otherwise.
+ * @remark *tokens may be NULL on output if NULL in input and no token is found
+ */
+AP_DECLARE(const char *) ap_parse_token_list_strict(apr_pool_t *p, const char *tok,
+                                                    apr_array_header_t **tokens,
+                                                    int skip_invalid);
+
 /**
  * Retrieve a token, spacing over it and adjusting the pointer to
  * the first non-white byte afterwards.  Note that these tokens
index caf1cae99fa6dd2b153f3b43148d70a0b17fa6af..5f696691ae2102ed64a75e7ab943c9a64656d723 100644 (file)
@@ -1356,6 +1356,7 @@ int ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
          */
         if (apr_date_checkmask(buffer, "HTTP/#.# ###*")) {
             int major, minor;
+            int toclose;
 
             major = buffer[5] - '0';
             minor = buffer[7] - '0';
@@ -1466,7 +1467,12 @@ int ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             te = apr_table_get(r->headers_out, "Transfer-Encoding");
 
             /* strip connection listed hop-by-hop headers from response */
-            backend->close = ap_proxy_clear_connection_fn(r, r->headers_out);
+            toclose = ap_proxy_clear_connection_fn(r, r->headers_out);
+            backend->close = (toclose != 0);
+            if (toclose < 0) {
+                return ap_proxyerror(r, HTTP_BAD_REQUEST,
+                        "Malformed connection header");
+            }
 
             if ((buf = apr_table_get(r->headers_out, "Content-Type"))) {
                 ap_set_content_type(r, apr_pstrdup(p, buf));
index d1b6352d5f0cb2302c77dae1faaa19ea4c0a6cb9..8861b2b3c1887ccc9c1c2b51466a43ecd6a37aa8 100644 (file)
@@ -3218,68 +3218,58 @@ PROXY_DECLARE(proxy_balancer_shared *) ap_proxy_find_balancershm(ap_slotmem_prov
 typedef struct header_connection {
     apr_pool_t *pool;
     apr_array_header_t *array;
-    const char *first;
-    unsigned int closed:1;
+    const char *error;
+    int is_req;
 } header_connection;
 
 static int find_conn_headers(void *data, const char *key, const char *val)
 {
     header_connection *x = data;
-    const char *name;
-
-    do {
-        while (*val == ',') {
-            val++;
-        }
-        name = ap_get_token(x->pool, &val, 0);
-        if (!strcasecmp(name, "close")) {
-            x->closed = 1;
-        }
-        if (!x->first) {
-            x->first = name;
-        }
-        else {
-            const char **elt;
-            if (!x->array) {
-                x->array = apr_array_make(x->pool, 4, sizeof(char *));
-            }
-            elt = apr_array_push(x->array);
-            *elt = name;
-        }
-    } while (*val);
-
-    return 1;
+    x->error = ap_parse_token_list_strict(x->pool, val, &x->array, !x->is_req);
+    return !x->error;
 }
 
 /**
  * Remove all headers referred to by the Connection header.
+ * Returns -1 on error. Otherwise, returns 1 if 'Close' was seen in
+ * the Connection header tokens, and 0 if not.
  */
 static int ap_proxy_clear_connection(request_rec *r, apr_table_t *headers)
 {
-    const char **name;
+    int closed = 0;
     header_connection x;
 
     x.pool = r->pool;
     x.array = NULL;
-    x.first = NULL;
-    x.closed = 0;
+    x.error = NULL;
+    x.is_req = (headers == r->headers_in);
 
     apr_table_unset(headers, "Proxy-Connection");
 
     apr_table_do(find_conn_headers, &x, headers, "Connection", NULL);
-    if (x.first) {
-        /* fast path - no memory allocated for one header */
-        apr_table_unset(headers, "Connection");
-        apr_table_unset(headers, x.first);
+    apr_table_unset(headers, "Connection");
+
+    if (x.error) {
+        ap_log_rerror(APLOG_MARK, APLOG_NOTICE, 0, r, APLOGNO()
+                "Error parsing Connection header: %s", x.error);
+        return -1;
     }
+
     if (x.array) {
-        /* two or more headers */
-        while ((name = apr_array_pop(x.array))) {
-            apr_table_unset(headers, *name);
+        int i;
+        for (i = 0; i < x.array->nelts; i++) {
+            const char *name = APR_ARRAY_IDX(x.array, i, const char *);
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
+                          "Removing header '%s' listed in Connection header",
+                          name);
+            if (!strcasecmp(name, "close")) {
+                closed = 1;
+            }
+            apr_table_unset(headers, name);
         }
     }
 
-    return x.closed;
+    return closed;
 }
 
 PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
@@ -3487,7 +3477,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     }
 
     proxy_run_fixups(r);
-    ap_proxy_clear_connection(r, r->headers_in);
+    if (ap_proxy_clear_connection(r, r->headers_in) < 0) {
+        return HTTP_BAD_REQUEST;
+    }
 
     /* send request headers */
     headers_in_array = apr_table_elts(r->headers_in);
index 42b0d2dad7c84d5ced17ccab25728a7f21bf3f21..c960248f7e082a1b9691d834c3ca182d4fd90cca 100644 (file)
@@ -1452,6 +1452,95 @@ AP_DECLARE(int) ap_find_etag_weak(apr_pool_t *p, const char *line,
     return find_list_item(p, line, tok, AP_ETAG_WEAK);
 }
 
+/* Grab a list of tokens of the format 1#token (from RFC7230) */
+AP_DECLARE(const char *) ap_parse_token_list_strict(apr_pool_t *p,
+                                                const char *str_in,
+                                                apr_array_header_t **tokens,
+                                                int skip_invalid)
+{
+    int in_leading_space = 1;
+    int in_trailing_space = 0;
+    int string_end = 0;
+    const char *tok_begin;
+    const char *cur;
+
+    if (!str_in) {
+        return NULL;
+    }
+
+    tok_begin = cur = str_in;
+
+    while (!string_end) {
+        const unsigned char c = (unsigned char)*cur;
+
+        if (!TEST_CHAR(c, T_HTTP_TOKEN_STOP) && c != '\0') {
+            /* Non-separator character; we are finished with leading
+             * whitespace. We must never have encountered any trailing
+             * whitespace before the delimiter (comma) */
+            in_leading_space = 0;
+            if (in_trailing_space) {
+                return "Encountered illegal whitespace in token";
+            }
+        }
+        else if (c == ' ' || c == '\t') {
+            /* "Linear whitespace" only includes ASCII CRLF, space, and tab;
+             * we can't get a CRLF since headers are split on them already,
+             * so only look for a space or a tab */
+            if (in_leading_space) {
+                /* We're still in leading whitespace */
+                ++tok_begin;
+            }
+            else {
+                /* We must be in trailing whitespace */
+                ++in_trailing_space;
+            }
+        }
+        else if (c == ',' || c == '\0') {
+            if (!in_leading_space) {
+                /* If we're out of the leading space, we know we've read some
+                 * characters of a token */
+                if (*tokens == NULL) {
+                    *tokens = apr_array_make(p, 4, sizeof(char *));
+                }
+                APR_ARRAY_PUSH(*tokens, char *) =
+                    apr_pstrmemdup((*tokens)->pool, tok_begin,
+                                   (cur - tok_begin) - in_trailing_space);
+            }
+            /* We're allowed to have null elements, just don't add them to the
+             * array */
+
+            tok_begin = cur + 1;
+            in_leading_space = 1;
+            in_trailing_space = 0;
+            string_end = (c == '\0');
+        }
+        else {
+            /* Encountered illegal separator char */
+            if (skip_invalid) {
+                /* Skip to the next separator */
+                const char *temp;
+                temp = ap_strchr_c(cur, ',');
+                if(!temp) {
+                    temp = ap_strchr_c(cur, '\0');
+                }
+
+                /* Act like we haven't seen a token so we reset */
+                cur = temp - 1;
+                in_leading_space = 1;
+                in_trailing_space = 0;
+            }
+            else {
+                return apr_psprintf(p, "Encountered illegal separator "
+                                    "'\\x%.2x'", (unsigned int)c);
+            }
+        }
+
+        ++cur;
+    }
+
+    return NULL;
+}
+
 /* Retrieve a token, spacing over it and returning a pointer to
  * the first non-white byte afterwards.  Note that these tokens
  * are delimited by semis and commas; and can also be delimited