File apache2-CVE-2022-37436.patch of Package apache2

Overview Repositories Revisions Requests Users Attributes Meta

File apache2-CVE-2022-37436.patch of Package apache2 (Revision fc057ef4d3f63b1b98cad224011f2876)

Currently displaying revision fc057ef4d3f63b1b98cad224011f2876 , Show latest

From 2192bd4200083a0d20bf601c2fc9d635e7e4dbfc Mon Sep 17 00:00:00 2001
From: Eric Covener <covener@apache.org>
Date: Tue, 10 Jan 2023 13:18:26 +0000
Subject: [PATCH] fail on bad header

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1906539 13f79535-47bb-0310-9956-ffa450edef68
---
 modules/proxy/mod_proxy_http.c | 46 ++++++++++++++++++++--------------
 server/protocol.c              |  2 ++
 2 files changed, 29 insertions(+), 19 deletions(-)

Index: httpd-2.4.33/modules/proxy/mod_proxy_http.c
===================================================================
--- httpd-2.4.33.orig/modules/proxy/mod_proxy_http.c
+++ httpd-2.4.33/modules/proxy/mod_proxy_http.c
@@ -28,6 +28,9 @@ static apr_status_t ap_proxy_http_cleanu
                                           request_rec *r,
                                           proxy_conn_rec *backend);
 
+static apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n,
+                                    request_rec *r, int flags, int *read);
+
 /*
  * Canonicalise http-like URLs.
  *  scheme is the scheme for the URL
@@ -1022,17 +1025,18 @@ static void process_proxy_header(request
  * any sense at all, since we depend on buffer still containing
  * what was read by ap_getline() upon return.
  */
-static void ap_proxy_read_headers(request_rec *r, request_rec *rr,
+static apr_status_t ap_proxy_read_headers(request_rec *r, request_rec *rr,
                                   char *buffer, int size,
                                   conn_rec *c, int *pread_len)
 {
     int len;
     char *value, *end;
-    char field[MAX_STRING_LEN];
     int saw_headers = 0;
     void *sconf = r->server->module_config;
     proxy_server_conf *psc;
     proxy_dir_conf *dconf;
+    apr_status_t rc;
+    apr_bucket_brigade *tmp_bb;
 
     dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
     psc = (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module);
@@ -1047,8 +1051,36 @@ static void ap_proxy_read_headers(reques
      */
     ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r,
                   "Headers received from backend:");
-    while ((len = ap_getline(buffer, size, rr, 1)) > 0) {
-        ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "%s", buffer);
+
+    tmp_bb = apr_brigade_create(r->pool, c->bucket_alloc);
+    while (1) {
+        rc = ap_proxygetline(tmp_bb, buffer, size, rr,
+                             AP_GETLINE_FOLD | AP_GETLINE_NOSPC_EOL, &len);
+
+
+        if (rc != APR_SUCCESS) {
+            if (APR_STATUS_IS_ENOSPC(rc)) {
+                int trunc = (len > 128 ? 128 : len) / 2;
+                ap_log_rerror(APLOG_MARK, APLOG_WARNING, rc, r, APLOGNO(10124)
+                        "header size is over the limit allowed by "
+                        "ResponseFieldSize (%d bytes). "
+                        "Bad response header: '%.*s[...]%s'",
+                        size, trunc, buffer, buffer + len - trunc);
+            }
+            else {
+                ap_log_rerror(APLOG_MARK, APLOG_WARNING, rc, r, APLOGNO(10404)
+                              "Error reading headers from backend");
+            }
+            r->headers_out = NULL;
+            return rc;
+        }
+
+        if (len <= 0) {
+            break;
+        }
+        else {
+            ap_log_rerror(APLOG_MARK, APLOG_TRACE4, 0, r, "%s", buffer);
+        }
 
         if (!(value = strchr(buffer, ':'))) {     /* Find the colon separator */
 
@@ -1067,7 +1099,7 @@ static void ap_proxy_read_headers(reques
                 if (psc->badopt == bad_error) {
                     /* Nope, it wasn't even an extra HTTP header. Give up. */
                     r->headers_out = NULL;
-                    return;
+                    return APR_EINVAL;
                 }
                 else if (psc->badopt == bad_body) {
                     /* if we've already started loading headers_out, then
@@ -1081,13 +1113,13 @@ static void ap_proxy_read_headers(reques
                                       "in headers returned by %s (%s)",
                                       r->uri, r->method);
                         *pread_len = len;
-                        return;
+                        return APR_SUCCESS;
                     }
                     else {
                         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01099)
                                       "No HTTP headers returned by %s (%s)",
                                       r->uri, r->method);
-                        return;
+                        return APR_SUCCESS;
                     }
                 }
             }
@@ -1116,17 +1148,8 @@ static void ap_proxy_read_headers(reques
          */
         process_proxy_header(r, dconf, buffer, value);
         saw_headers = 1;
-
-        /* the header was too long; at the least we should skip extra data */
-        if (len >= size - 1) {
-            while ((len = ap_getline(field, MAX_STRING_LEN, rr, 1))
-                    >= MAX_STRING_LEN - 1) {
-                /* soak up the extra data */
-            }
-            if (len == 0) /* time to exit the larger loop as well */
-                break;
-        }
     }
+    return APR_SUCCESS;
 }
 
 
@@ -1137,23 +1160,19 @@ static int addit_dammit(void *v, const c
     return 1;
 }
 
-static
-apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n, request_rec *r,
-                             int fold, int *writen)
+static apr_status_t ap_proxygetline(apr_bucket_brigade *bb, char *s, int n,
+                                    request_rec *r, int flags, int *read)
 {
-    char *tmp_s = s;
     apr_status_t rv;
     apr_size_t len;
 
-    rv = ap_rgetline(&tmp_s, n, &len, r, fold, bb);
+    rv = ap_rgetline(&s, n, &len, r, flags, bb);
     apr_brigade_cleanup(bb);
 
-    if (rv == APR_SUCCESS) {
-        *writen = (int) len;
-    } else if (APR_STATUS_IS_ENOSPC(rv)) {
-        *writen = n;
+    if (rv == APR_SUCCESS || APR_STATUS_IS_ENOSPC(rv)) {
+        *read = (int)len;
     } else {
-        *writen = -1;
+        *read = -1;
     }
 
     return rv;
@@ -1385,10 +1404,10 @@ apr_status_t ap_proxy_http_process_respo
                          "Set-Cookie", NULL);
 
             /* shove the headers direct into r->headers_out */
-            ap_proxy_read_headers(r, backend->r, buffer, sizeof(buffer), origin,
+            rc = ap_proxy_read_headers(r, backend->r, buffer, sizeof(buffer), origin,
                                   &pread_len);
 
-            if (r->headers_out == NULL) {
+            if (rc != APR_SUCCESS || r->headers_out == NULL) {
                 ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01106)
                               "bad HTTP/%d.%d header returned by %s (%s)",
                               major, minor, r->uri, r->method);
Index: httpd-2.4.33/server/protocol.c
===================================================================
--- httpd-2.4.33.orig/server/protocol.c
+++ httpd-2.4.33/server/protocol.c
@@ -224,9 +224,12 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
     int do_alloc = (*s == NULL), saw_eos = 0;
     int fold = flags & AP_GETLINE_FOLD;
     int crlf = flags & AP_GETLINE_CRLF;
+    int nospc_eol = flags & AP_GETLINE_NOSPC_EOL;
+    int saw_eol = 0, saw_nospc = 0;
 
     if (!n) {
         /* Needs room for NUL byte at least */
+        *read = 0;
         return APR_BADARG;
     }
 
@@ -238,7 +241,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
     if (last_char)
         *last_char = '\0';
 
-    for (;;) {
+    do {
         apr_brigade_cleanup(bb);
         rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_GETLINE,
                             APR_BLOCK_READ, 0);
@@ -282,8 +285,52 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
 
             /* Would this overrun our buffer?  If so, we'll die. */
             if (n < bytes_handled + len) {
-                rv = APR_ENOSPC;
-                goto cleanup;
+                /* Before we die, let's fill the buffer up to its limit (i.e.
+                 * fall through with the remaining length, if any), setting
+                 * saw_eol on LF to stop the outer loop appropriately; we may
+                 * come back here once the buffer is filled (no LF seen), and
+                 * either be done at that time or continue to wait for LF here
+                 * if nospc_eol is set.
+                 *
+                 * But there is also a corner cases which we want to address,
+                 * namely if the buffer is overrun by the final LF only (i.e.
+                 * the CR fits in); this is not really an overrun since we'll
+                 * strip the CR finally (and use it for NUL byte), but anyway
+                 * we have to handle the case so that it's not returned to the
+                 * caller as part of the truncated line (it's not!). This is
+                 * easier to consider that LF is out of counting and thus fall
+                 * through with no error (saw_eol is set to 2 so that we later
+                 * ignore LF handling already done here), while folding and
+                 * nospc_eol logics continue to work (or fail) appropriately.
+                 */
+                saw_eol = (str[len - 1] == APR_ASCII_LF);
+                if (/* First time around */
+                    saw_eol && !saw_nospc
+                    /*  Single LF completing the buffered CR, */
+                    && ((len == 1 && ((*s)[bytes_handled - 1] == APR_ASCII_CR))
+                    /*  or trailing CRLF overuns by LF only */
+                        || (len > 1 && str[len - 2] == APR_ASCII_CR
+                            && n - bytes_handled + 1 == len))) {
+                    /* In both cases *last_char is (to be) the CR stripped by
+                     * later 'bytes_handled = last_char - *s'.
+                     */
+                    saw_eol = 2;
+                }
+                else {
+                    /* In any other case we'd lose data. */
+                    rv = APR_ENOSPC;
+                    saw_nospc = 1;
+                }
+                len = n - bytes_handled;
+                if (!len) {
+                    if (saw_eol) {
+                        break;
+                    }
+                    if (nospc_eol) {
+                        continue;
+                    }
+                    goto cleanup;
+                }
             }
 
             /* Do we have to handle the allocation ourselves? */
@@ -322,18 +369,28 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
 
         /* If we got a full line of input, stop reading */
         if (last_char && (*last_char == APR_ASCII_LF)) {
-            break;
+            saw_eol = 1;
         }
+    } while (!saw_eol);
+
+    if (rv != APR_SUCCESS) {
+        /* End of line after APR_ENOSPC above */
+        goto cleanup;
     }
 
     /* 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--;
-    }
-    else if (crlf) {
-        rv = APR_EINVAL;
-        goto cleanup;
+     * if the last-but-one character is a CR, terminate there.
+     * LF is handled above (not accounted) when saw_eol == 2,
+     * the last char is CR to terminate at still.
+     */
+    if (saw_eol < 2) {
+        if (last_char > *s && last_char[-1] == APR_ASCII_CR) {
+            last_char--;
+        }
+        else if (crlf) {
+            rv = APR_EINVAL;
+            goto cleanup;
+        }
     }
     bytes_handled = last_char - *s;
 
@@ -407,8 +464,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_cor
 
                     next_size = n - bytes_handled;
 
-                    rv = ap_rgetline_core(&tmp, next_size,
-                                          &next_len, r, 0, bb);
+                    rv = ap_rgetline_core(&tmp, next_size, &next_len, r,
+                                          flags & ~AP_GETLINE_FOLD, bb);
                     if (rv != APR_SUCCESS) {
                         goto cleanup;
                     }
@@ -442,22 +499,22 @@ cleanup:
     if (bytes_handled >= n) {
         bytes_handled = n - 1;
     }
+
+    *read = bytes_handled;
     if (*s) {
         /* ensure the string is NUL terminated */
-        (*s)[bytes_handled] = '\0';
-    }
-    *read = bytes_handled;
-
-    if (rv != APR_SUCCESS) {
-        return rv;
-    }
+        (*s)[*read] = '\0';
 
-    /* PR#43039: We shouldn't accept NULL bytes within the line */
-    if (strlen(*s) < bytes_handled) {
-        return APR_EINVAL;
+        /* PR#43039: We shouldn't accept NULL bytes within the line */
+        bytes_handled = strlen(*s);
+        if (bytes_handled < *read) {
+            *read = bytes_handled;
+            if (rv == APR_SUCCESS) {
+                rv = APR_EINVAL;
+            }
+        }
     }
-
-    return APR_SUCCESS;
+    return rv;
 }
 
 #if APR_CHARSET_EBCDIC
Index: httpd-2.4.33/include/http_protocol.h
===================================================================
--- httpd-2.4.33.orig/include/http_protocol.h
+++ httpd-2.4.33/include/http_protocol.h
@@ -634,6 +634,8 @@ AP_CORE_DECLARE(void) ap_parse_uri(reque
 
 #define AP_GETLINE_FOLD 1 /* Whether to merge continuation lines */
 #define AP_GETLINE_CRLF 2 /*Whether line ends must be in the form CR LF */
+#define AP_GETLINE_NOSPC_EOL 4 /* Whether to consume up to and including the
+                                  end of line on APR_ENOSPC */
 
 /**
  * Get the next line of input for the request

Places

File apache2-CVE-2022-37436.patch of Package apache2 (Revision fc057ef4d3f63b1b98cad224011f2876)

Places