From: Yann Ylavic <ylavic@apache.org>
Date: Wed, 31 Aug 2016 19:49:25 +0000 (+0000)
Subject: Merge r892678, r1100511, r1102124 from trunk:
X-Git-Tag: 2.2.32~82
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=798127409001dd313a42a285ef53ee998f495ef4;p=thirdparty%2Fapache%2Fhttpd.git

Merge r892678, r1100511, r1102124 from trunk:

Reject requests containing (invalid) NULL characters in request line
or request headers.
PR 43039


use APR_STATUS_IS_TIMEUP() instead of direct comparison with APR_TIMEUP.


Use APR_STATUS_IS_... in some more cases.

While this is not strictly necessary everywhere, it makes it much easier
to find the problematic cases.


Submitted by: niq, covener, sf
Reviewed  by: wrowe, covener, ylavic


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@1758671 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index 5154da64834..07c02d03d60 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,9 @@ Changes with Apache 2.2.32
   *) core: CVE-2016-5387: Mitigate [f]cgi "httpoxy" issues.
      [Dominic Scheirlinck <dominic vendhq.com>, Yann Ylavic]
 
+  *) Core: reject NULLs in request line or request headers.
+     PR 43039 [Nick Kew]
+
   *) mod_ssl: Fix a possible memory leak on restart for custom [EC]DH params.
      [Jan Kaluza, Yann Ylavic]
 
diff --git a/STATUS b/STATUS
index f60d7facaf3..3fc37d54a10 100644
--- a/STATUS
+++ b/STATUS
@@ -103,19 +103,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) core: Reject requests containing (invalid) NULL characters in request line
-     or request headers. (Including embedded %00 in URL).
-     (Use APR_STATUS_IS_... in some more cases.)
-     Trunk version of patch
-         http://svn.apache.org/r892678
-         http://svn.apache.org/r1100511
-         http://svn.apache.org/r1102124
-     Backport: (trunk works as well)
-         https://raw.githubusercontent.com/wrowe/patches/master/backport-2.2.x-r892678.patch
-     Submitted by niq, status legibility fixes by covener, sf
-     PR: 43039
-     +1: wrowe, covener, ylavic
-
   *) core: Limit to ten the number of tolerated empty lines between request.
      Before this commit, the maximum number of empty lines was the same as
      configured LimitRequestFields, defaulting to 100, which was way too much.
diff --git a/server/protocol.c b/server/protocol.c
index e9611a142a0..c3a034e504d 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -433,8 +433,13 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
             }
         }
     }
-
     *read = bytes_handled;
+
+    /* PR#43039: We shouldn't accept NULL bytes within the line */
+    if (strlen(*s) < bytes_handled - 1) {
+        return APR_EINVAL;
+    }
+
     return APR_SUCCESS;
 }
 
@@ -597,12 +602,15 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
              * buffer before finding the end-of-line.  This is only going to
              * happen if it exceeds the configured limit for a request-line.
              */
-            if (rv == APR_ENOSPC) {
+            if (APR_STATUS_IS_ENOSPC(rv)) {
                 r->status    = HTTP_REQUEST_URI_TOO_LARGE;
             }
             else if (APR_STATUS_IS_TIMEUP(rv)) {
                 r->status = HTTP_REQUEST_TIME_OUT;
             }
+            else if (APR_STATUS_IS_EINVAL(rv)) {
+                r->status = HTTP_BAD_REQUEST;
+            }
             r->proto_num = HTTP_VERSION(1,0);
             r->protocol  = apr_pstrdup(r->pool, "HTTP/1.0");
             return 0;
@@ -908,9 +916,16 @@ request_rec *ap_read_request(conn_rec *conn)
 
     /* Get the request... */
     if (!read_request_line(r, tmp_bb)) {
-        if (r->status == HTTP_REQUEST_URI_TOO_LARGE) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                          "request failed: URI too long (longer than %d)", r->server->limit_req_line);
+        if (r->status == HTTP_REQUEST_URI_TOO_LARGE
+            || r->status == HTTP_BAD_REQUEST) {
+            if (r->status == HTTP_BAD_REQUEST) {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                              "request failed: invalid characters in URI");
+            }
+            else {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                              "request failed: URI too long (longer than %d)", r->server->limit_req_line);
+            }
             ap_send_error_response(r, 0);
             ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
             ap_run_log_transaction(r);