Correctly align the behavior of headers_in to be consistent with the

treatment of headers_out, resolving PR 48359 by keeping subrequest
scope changes out of the main request headers.  This ensures that all
requests-without-bodies behave as the requests-with-bodies code has.

Mitre: CVE-2010-0434

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

diff --git a/CHANGES b/CHANGES
index f2e79f286e44799a71cdf69c1ed80753fd35d6c9..3383fe692dc4d2efe47dcaec07197addccbf1317 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,11 @@
 
 Changes with Apache 2.3.5
 
+  *) Ensure each subrequest has a shallow copy of headers_in so that the
+     parent request headers are not corrupted.  Elimiates a problematic
+     optimization in the case of no request body.  PR 48359 
+     [Jake Scott, William Rowe]
+
   *) Turn static function get_server_name_for_url() into public
      ap_get_server_name_for_url() and use it where appropriate. This
      fixes mod_rewrite generating invalid URLs for redirects to IPv6
@@ -23,9 +28,6 @@ Changes with Apache 2.3.5
 
   *) mod_proxy_balancer: Fix crash in balancer-manager. [Rainer Jung]
 
-  *) mod_headers: Ensure that changes to the main request remain valid when
-     the subrequest is destroyed. PR 48359 [Nick Kew, Ruediger Pluem]
-
   *) Core HTTP: disable keepalive when the Client has sent
      Expect: 100-continue
      but we respond directly with a non-100 response.

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index 869bbc82a40a03443210e00200632753362e8e6d..a12cbf4c76a3c526bb213bae5300c09482690225 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -442,7 +442,7 @@ static request_rec *internal_internal_redirect(const char *new_uri,
     new->request_time    = r->request_time;
     new->main            = r->main;
 
-    new->headers_in      = r->headers_in;
+    new->headers_in      = apr_table_copy(r->pool, r->headers_in);
     new->headers_out     = apr_table_make(r->pool, 12);
     new->err_headers_out = r->err_headers_out;
     new->subprocess_env  = rename_original_env(r->pool, r->subprocess_env);
@@ -515,6 +515,8 @@ AP_DECLARE(void) ap_internal_fast_redirect(request_rec *rr, request_rec *r)
     r->per_dir_config = rr->per_dir_config;
     /* copy output headers from subrequest, but leave negotiation headers */
     r->notes = apr_table_overlay(r->pool, rr->notes, r->notes);
+    r->headers_in = apr_table_overlay(r->pool, rr->headers_in,
+                                      r->headers_in);
     r->headers_out = apr_table_overlay(r->pool, rr->headers_out,
                                        r->headers_out);
     r->err_headers_out = apr_table_overlay(r->pool, rr->err_headers_out,

diff --git a/modules/metadata/mod_headers.c b/modules/metadata/mod_headers.c
index 5e7834aecc81643c306a825715835392bed0bf58..ad6fde714885ee2a6c20a1418974fa35af94c1d9 100644 (file)
--- a/modules/metadata/mod_headers.c
+++ b/modules/metadata/mod_headers.c
@@ -547,7 +547,7 @@ static const char *header_cmd(cmd_parms *cmd, void *indirconf,
  * Concatenate the return from each handler into one string that is
  * returned from this call.
  */
-static char* process_tags(header_entry *hdr, request_rec *r, request_rec *rr)
+static char* process_tags(header_entry *hdr, request_rec *r)
 {
     int i;
     const char *s;
@@ -558,9 +558,9 @@ static char* process_tags(header_entry *hdr, request_rec *r, request_rec *rr)
     for (i = 0; i < hdr->ta->nelts; i++) {
         s = tag[i].func(r, tag[i].arg);
         if (str == NULL)
-            str = apr_pstrdup(rr->pool, s);
+            str = apr_pstrdup(r->pool, s);
         else
-            str = apr_pstrcat(rr->pool, str, s, NULL);
+            str = apr_pstrcat(r->pool, str, s, NULL);
     }
     return str ? str : "";
 }
@@ -627,12 +627,6 @@ static void do_headers_fixup(request_rec *r, apr_table_t *headers,
     echo_do v;
     int i;
     const char *val;
-    request_rec *rr;
-
-    rr = r;
-    while (rr->main != NULL) {
-        rr = rr->main;
-    }
 
     for (i = 0; i < fixup->nelts; ++i) {
         header_entry *hdr = &((header_entry *) (fixup->elts))[i];
@@ -673,17 +667,17 @@ static void do_headers_fixup(request_rec *r, apr_table_t *headers,
 
         switch (hdr->action) {
         case hdr_add:
-            apr_table_addn(headers, hdr->header, process_tags(hdr, r, rr));
+            apr_table_addn(headers, hdr->header, process_tags(hdr, r));
             break;
         case hdr_append:
-            apr_table_mergen(headers, hdr->header, process_tags(hdr, r, rr));
+            apr_table_mergen(headers, hdr->header, process_tags(hdr, r));
             break;
         case hdr_merge:
             val = apr_table_get(headers, hdr->header);
             if (val == NULL) {
-                apr_table_addn(headers, hdr->header, process_tags(hdr, r, rr));
+                apr_table_addn(headers, hdr->header, process_tags(hdr, r));
             } else {
-                char *new_val = process_tags(hdr, r, rr);
+                char *new_val = process_tags(hdr, r);
                 apr_size_t new_val_len = strlen(new_val);
                 int tok_found = 0;
 
@@ -720,9 +714,9 @@ static void do_headers_fixup(request_rec *r, apr_table_t *headers,
             break;
         case hdr_set:
             if (!strcasecmp(hdr->header, "Content-Type")) {
-                 ap_set_content_type(r, process_tags(hdr, r, rr));
+                 ap_set_content_type(r, process_tags(hdr, r));
             }
-            apr_table_setn(headers, hdr->header, process_tags(hdr, r, rr));
+            apr_table_setn(headers, hdr->header, process_tags(hdr, r));
             break;
         case hdr_unset:
             apr_table_unset(headers, hdr->header);
@@ -742,7 +736,7 @@ static void do_headers_fixup(request_rec *r, apr_table_t *headers,
             if (apr_table_get(headers, hdr->header)) {
                 edit_do ed;
 
-                ed.p = rr->pool;
+                ed.p = r->pool;
                 ed.hdr = hdr;
                 ed.t = apr_table_make(r->pool, 5);
                 apr_table_do(edit_header, (void *) &ed, headers, hdr->header,

diff --git a/server/protocol.c b/server/protocol.c
index 38840f5429d2cafbfb7d4948e4126899679f2266..8061dc7de1ad99a491285be40c8b352628443e48 100644 (file)
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -1074,15 +1074,13 @@ request_rec *ap_read_request(conn_rec *conn)
     return r;
 }
 
-/* if a request with a body creates a subrequest, clone the original request's
- * input headers minus any headers pertaining to the body which has already
- * been read.  out-of-line helper function for ap_set_sub_req_protocol.
+/* if a request with a body creates a subrequest, remove original request's
+ * input headers which pertain to the body which has already been read.
+ * out-of-line helper function for ap_set_sub_req_protocol.
  */
 
-static void clone_headers_no_body(request_rec *rnew,
-                                  const request_rec *r)
+static void strip_headers_request_body(request_rec *rnew)
 {
-    rnew->headers_in = apr_table_copy(rnew->pool, r->headers_in);
     apr_table_unset(rnew->headers_in, "Content-Encoding");
     apr_table_unset(rnew->headers_in, "Content-Language");
     apr_table_unset(rnew->headers_in, "Content-Length");
@@ -1116,15 +1114,14 @@ AP_DECLARE(void) ap_set_sub_req_protocol(request_rec *rnew,
 
     rnew->status          = HTTP_OK;
 
+    rnew->headers_in = apr_table_copy(rnew->pool, r->headers_in);
+
     /* did the original request have a body?  (e.g. POST w/SSI tags)
      * if so, make sure the subrequest doesn't inherit body headers
      */
     if (!r->kept_body && (apr_table_get(r->headers_in, "Content-Length")
         || apr_table_get(r->headers_in, "Transfer-Encoding"))) {
-        clone_headers_no_body(rnew, r);
-    } else {
-        /* no body (common case).  clone headers the cheap way */
-        rnew->headers_in      = r->headers_in;
+        strip_headers_request_body(rnew);
     }
     rnew->subprocess_env  = apr_table_copy(rnew->pool, r->subprocess_env);
     rnew->headers_out     = apr_table_make(rnew->pool, 5);