[apache2] 04/07: CVE-2018-1301: out of bound read after failure in reading HTTP request

Stefan Fritsch sf at moszumanska.debian.org
Sat Mar 31 09:46:39 UTC 2018


This is an automated email from the git hooks/post-receive script.

sf pushed a commit to branch jessie
in repository apache2.

commit f6824e7afed4183608661bbb4f4fdf26577f09cd
Author: Stefan Fritsch <sf at sfritsch.de>
Date:   Fri Mar 30 16:19:15 2018 +0200

    CVE-2018-1301: out of bound read after failure in reading HTTP request
---
 debian/changelog                                   |   2 +
 ...-2018-1301-HTTP-request-read-out-of-bounds.diff | 197 +++++++++++++++++++++
 debian/patches/series                              |   1 +
 3 files changed, 200 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index dccdd9a..10e76a6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -9,6 +9,8 @@ apache2 (2.4.10-10+deb8u12) UNRELEASED; urgency=medium
     newline characters. Behavior can be changed with new directive
     'RegexDefaultOptions'.
   * CVE-2018-1283: Tampering of mod_session data for CGI applications.
+  * CVE-2018-1301: Possible out of bound access after failure in reading the
+    HTTP request
 
  -- Stefan Fritsch <sf at debian.org>  Sat, 31 Mar 2018 11:24:46 +0200
 
diff --git a/debian/patches/CVE-2018-1301-HTTP-request-read-out-of-bounds.diff b/debian/patches/CVE-2018-1301-HTTP-request-read-out-of-bounds.diff
new file mode 100644
index 0000000..6c0485e
--- /dev/null
+++ b/debian/patches/CVE-2018-1301-HTTP-request-read-out-of-bounds.diff
@@ -0,0 +1,197 @@
+# CVE-2018-1301
+# https://svn.apache.org/r1824469
+--- apache2.orig/server/protocol.c
++++ apache2/server/protocol.c
+@@ -222,6 +222,11 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+     int fold = flags & AP_GETLINE_FOLD;
+     int crlf = flags & AP_GETLINE_CRLF;
+ 
++    if (!n) {
++        /* Needs room for NUL byte at least */
++        return APR_BADARG;
++    }
++
+     /*
+      * Initialize last_char as otherwise a random value will be compared
+      * against APR_ASCII_LF at the end of the loop if bb only contains
+@@ -235,14 +240,15 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+         rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_GETLINE,
+                             APR_BLOCK_READ, 0);
+         if (rv != APR_SUCCESS) {
+-            return rv;
++            goto cleanup;
+         }
+ 
+         /* Something horribly wrong happened.  Someone didn't block! 
+          * (this also happens at the end of each keepalive connection)
+          */
+         if (APR_BRIGADE_EMPTY(bb)) {
+-            return APR_EGENERAL;
++            rv = APR_EGENERAL;
++            goto cleanup;
+         }
+ 
+         for (e = APR_BRIGADE_FIRST(bb);
+@@ -260,7 +266,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+ 
+             rv = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
+             if (rv != APR_SUCCESS) {
+-                return rv;
++                goto cleanup;
+             }
+ 
+             if (len == 0) {
+@@ -273,17 +279,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+ 
+             /* Would this overrun our buffer?  If so, we'll die. */
+             if (n < bytes_handled + len) {
+-                *read = bytes_handled;
+-                if (*s) {
+-                    /* ensure this string is NUL terminated */
+-                    if (bytes_handled > 0) {
+-                        (*s)[bytes_handled-1] = '\0';
+-                    }
+-                    else {
+-                        (*s)[0] = '\0';
+-                    }
+-                }
+-                return APR_ENOSPC;
++                rv = APR_ENOSPC;
++                goto cleanup;
+             }
+ 
+             /* Do we have to handle the allocation ourselves? */
+@@ -291,7 +288,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+                 /* We'll assume the common case where one bucket is enough. */
+                 if (!*s) {
+                     current_alloc = len;
+-                    *s = apr_palloc(r->pool, current_alloc);
++                    *s = apr_palloc(r->pool, current_alloc + 1);
+                 }
+                 else if (bytes_handled + len > current_alloc) {
+                     /* Increase the buffer size */
+@@ -302,7 +299,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+                         new_size = (bytes_handled + len) * 2;
+                     }
+ 
+-                    new_buffer = apr_palloc(r->pool, new_size);
++                    new_buffer = apr_palloc(r->pool, new_size + 1);
+ 
+                     /* Copy what we already had. */
+                     memcpy(new_buffer, *s, bytes_handled);
+@@ -326,19 +323,15 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+         }
+     }
+ 
+-    if (crlf && (last_char <= *s || last_char[-1] != APR_ASCII_CR)) {
+-        *last_char = '\0';
+-        bytes_handled = last_char - *s;
+-        *read = bytes_handled;
+-        return APR_EINVAL;
+-    }
+-
+-    /* Now NUL-terminate the string at the end of the line;
++    /* Now terminate the string at the end of the line;
+      * if the last-but-one character is a CR, terminate there */
+     if (last_char > *s && last_char[-1] == APR_ASCII_CR) {
+         last_char--;
+     }
+-    *last_char = '\0';
++    else if (crlf) {
++        rv = APR_EINVAL;
++        goto cleanup;
++    }
+     bytes_handled = last_char - *s;
+ 
+     /* If we're folding, we have more work to do.
+@@ -358,7 +351,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+             rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_SPECULATIVE,
+                                 APR_BLOCK_READ, 1);
+             if (rv != APR_SUCCESS) {
+-                return rv;
++                goto cleanup;
+             }
+ 
+             if (APR_BRIGADE_EMPTY(bb)) {
+@@ -375,7 +368,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+             rv = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
+             if (rv != APR_SUCCESS) {
+                 apr_brigade_cleanup(bb);
+-                return rv;
++                goto cleanup;
+             }
+ 
+             /* Found one, so call ourselves again to get the next line.
+@@ -392,10 +385,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+             if (c == APR_ASCII_BLANK || c == APR_ASCII_TAB) {
+                 /* Do we have enough space? We may be full now. */
+                 if (bytes_handled >= n) {
+-                    *read = n;
+-                    /* ensure this string is terminated */
+-                    (*s)[n-1] = '\0';
+-                    return APR_ENOSPC;
++                    rv = APR_ENOSPC;
++                    goto cleanup;
+                 }
+                 else {
+                     apr_size_t next_size, next_len;
+@@ -408,7 +399,6 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+                         tmp = NULL;
+                     }
+                     else {
+-                        /* We're null terminated. */
+                         tmp = last_char;
+                     }
+ 
+@@ -417,7 +407,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+                     rv = ap_rgetline_core(&tmp, next_size,
+                                           &next_len, r, 0, bb);
+                     if (rv != APR_SUCCESS) {
+-                        return rv;
++                        goto cleanup;
+                     }
+ 
+                     if (do_alloc && next_len > 0) {
+@@ -431,7 +421,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+                         memcpy(new_buffer, *s, bytes_handled);
+ 
+                         /* copy the new line, including the trailing null */
+-                        memcpy(new_buffer + bytes_handled, tmp, next_len + 1);
++                        memcpy(new_buffer + bytes_handled, tmp, next_len);
+                         *s = new_buffer;
+                     }
+ 
+@@ -444,8 +434,21 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
+             }
+         }
+     }
++
++cleanup:
++    if (bytes_handled >= n) {
++        bytes_handled = n - 1;
++    }
++    if (*s) {
++        /* ensure the string is NUL terminated */
++        (*s)[bytes_handled] = '\0';
++    }
+     *read = bytes_handled;
+ 
++    if (rv != APR_SUCCESS) {
++        return rv;
++    }
++
+     /* PR#43039: We shouldn't accept NULL bytes within the line */
+     if (strlen(*s) < bytes_handled) {
+         return APR_EINVAL;
+@@ -484,6 +487,11 @@ AP_DECLARE(int) ap_getline(char *s, int
+     apr_size_t len;
+     apr_bucket_brigade *tmp_bb;
+ 
++    if (n < 1) {
++        /* Can't work since we always NUL terminate */
++        return -1;
++    }
++
+     tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+     rv = ap_rgetline(&tmp_s, n, &len, r, flags, tmp_bb);
+     apr_brigade_destroy(tmp_bb);
diff --git a/debian/patches/series b/debian/patches/series
index a8ca06b..bbd5b82 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -35,3 +35,4 @@ core-Disallow-Methods-registration-at-run-time-.htac.patch
 CVE-2017-15710-mod_authnz_ldap.diff
 CVE-2017-15715-regex-line-endings.diff
 CVE-2018-1283-mod_session.diff
+CVE-2018-1301-HTTP-request-read-out-of-bounds.diff

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-apache/apache2.git



More information about the Pkg-apache-commits mailing list