From 58ee1df31fa9e9af106aaeabb82374c36b433822 Mon Sep 17 00:00:00 2001 From: Eric Covener Date: Tue, 25 Jul 2023 14:36:11 +0200 Subject: [PATCH] apache2/mod_proxy_uwsgi: stricter backend HTTP response parsing/validation HTTP Response Smuggling vulnerability in Apache HTTP Server via mod_proxy_uwsgi. Special characters in the origin response header can truncate/split the response forwarded to the client. Fix #2538 origin: https://github.com/apache/httpd/commit/d753ea76b5972a85349b68c31b59d04c60014f2d.patch bug-cve: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-27522 --- apache2/mod_proxy_uwsgi.c | 48 ++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/apache2/mod_proxy_uwsgi.c b/apache2/mod_proxy_uwsgi.c index 507262bed..026e63e03 100644 --- a/apache2/mod_proxy_uwsgi.c +++ b/apache2/mod_proxy_uwsgi.c @@ -301,18 +301,16 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec *backend, proxy_server_ apr_bucket_brigade *pass_bb = apr_brigade_create(r->pool, c->bucket_alloc); len = ap_getline(buffer, sizeof(buffer), rp, 1); - if (len <= 0) { - // oops + /* invalid or empty */ return HTTP_INTERNAL_SERVER_ERROR; } - backend->worker->s->read += len; - - if (len >= sizeof(buffer)-1) { - // oops + if ((apr_size_t)len >= sizeof(buffer)) { + /* too long */ return HTTP_INTERNAL_SERVER_ERROR; } + /* Position of http status code */ int status_start; if (apr_date_checkmask(buffer, "HTTP/#.# ###*")) { @@ -320,8 +318,8 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec *backend, proxy_server_ } else if (apr_date_checkmask(buffer, "HTTP/# ###*")) { status_start = 7; } else { - // oops - return HTTP_INTERNAL_SERVER_ERROR; + /* not HTTP */ + return HTTP_BAD_GATEWAY; } int status_end = status_start + 3; @@ -340,17 +338,41 @@ static int uwsgi_response(request_rec *r, proxy_conn_rec *backend, proxy_server_ } r->status_line = apr_pstrdup(r->pool, &buffer[status_start]); - // start parsing headers; + /* parse headers */ while ((len = ap_getline(buffer, sizeof(buffer), rp, 1)) > 0) { + if ((apr_size_t)len >= sizeof(buffer)) { + /* too long */ + len = -1; + break; + } value = strchr(buffer, ':'); - // invalid header skip - if (!value) continue; - *value = '\0'; - ++value; + if (!value) { + /* invalid header */ + len = -1; + break; + } + *value++ = '\0'; + if (*ap_scan_http_token(buffer)) { + /* invalid name */ + len = -1; + break; + } while (apr_isspace(*value)) ++value; for (end = &value[strlen(value)-1]; end > value && apr_isspace(*end); --end) *end = '\0'; + if (*ap_scan_http_field_content(value)) { + /* invalid value */ + len = -1; + break; + } apr_table_add(r->headers_out, buffer, value); } + if (len < 0) { + /* Reset headers, but not to NULL because things below the chain expect + * this to be non NULL e.g. the ap_content_length_filter. + */ + r->headers_out = apr_table_make(r->pool, 1); + return HTTP_BAD_GATEWAY; + } if ((buf = apr_table_get(r->headers_out, "Content-Type"))) { ap_set_content_type(r, apr_pstrdup(r->pool, buf));