pool/apache2
SHA256
17
0
Fork 1
Code Issues Pull Requests Activity
Files
slfo-main
apache2/CVE-2025-53020.patch

497 lines
20 KiB
Diff
Raw Permalink Blame History

From ef98f4f494ff2f99d736a3716cd31219688b46f5 Mon Sep 17 00:00:00 2001
From: Eric Covener <covener@apache.org>
Date: Mon, 7 Jul 2025 12:12:49 +0000
Subject: [PATCH] backport 1927038 from trunk
improve h2 header error handling
Rewviewed By: icing, covener, rpluem
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1927046 13f79535-47bb-0310-9956-ffa450edef68
---
modules/http2/h2_request.c | 12 ++--
modules/http2/h2_request.h | 8 ++-
modules/http2/h2_session.c | 31 ++++++++-
modules/http2/h2_session.h | 3 +
modules/http2/h2_stream.c | 69 ++++++++++++-------
modules/http2/h2_util.c | 51 ++++++++------
modules/http2/h2_util.h | 11 ++-
test/modules/http2/test_200_header_invalid.py | 4 +-
8 files changed, 130 insertions(+), 59 deletions(-)
diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c
index 2713947c377..6373e0a244d 100644
--- a/modules/http2/h2_request.c
+++ b/modules/http2/h2_request.c
@@ -64,18 +64,20 @@ typedef struct {
apr_table_t *headers;
apr_pool_t *pool;
apr_status_t status;
+ h2_hd_scratch *scratch;
} h1_ctx;
static int set_h1_header(void *ctx, const char *key, const char *value)
{
h1_ctx *x = ctx;
int was_added;
- h2_req_add_header(x->headers, x->pool, key, strlen(key), value, strlen(value), 0, &was_added);
+ h2_req_add_header(x->headers, x->pool, key, strlen(key),
+ value, strlen(value), x->scratch, &was_added);
return 1;
}
apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
- request_rec *r)
+ request_rec *r, h2_hd_scratch *scratch)
{
h2_request *req;
const char *scheme, *authority, *path;
@@ -125,6 +127,7 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
x.pool = pool;
x.headers = req->headers;
x.status = APR_SUCCESS;
+ x.scratch = scratch;
apr_table_do(set_h1_header, &x, r->headers_in, NULL);
*preq = req;
@@ -134,7 +137,8 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
const char *name, size_t nlen,
const char *value, size_t vlen,
- size_t max_field_len, int *pwas_added)
+ struct h2_hd_scratch *scratch,
+ int *pwas_added)
{
apr_status_t status = APR_SUCCESS;
@@ -185,7 +189,7 @@ apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
else {
/* non-pseudo header, add to table */
status = h2_req_add_header(req->headers, pool, name, nlen, value, vlen,
- max_field_len, pwas_added);
+ scratch, pwas_added);
}
return status;
diff --git a/modules/http2/h2_request.h b/modules/http2/h2_request.h
index 7e20b697246..ae6b6a2510c 100644
--- a/modules/http2/h2_request.h
+++ b/modules/http2/h2_request.h
@@ -19,17 +19,21 @@
#include "h2.h"
+struct h2_hd_scratch;
+
h2_request *h2_request_create(int id, apr_pool_t *pool, const char *method,
const char *scheme, const char *authority,
const char *path, apr_table_t *header);
apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
- request_rec *r);
+ request_rec *r,
+ struct h2_hd_scratch *scratch);
apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
const char *name, size_t nlen,
const char *value, size_t vlen,
- size_t max_field_len, int *pwas_added);
+ struct h2_hd_scratch *scratch,
+ int *pwas_added);
apr_status_t h2_request_add_trailer(h2_request *req, apr_pool_t *pool,
const char *name, size_t nlen,
diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index fc8b6119ae8..a5f1872bc20 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -109,13 +109,29 @@ static void cleanup_unprocessed_streams(h2_session *session)
h2_mplx_c1_streams_do(session->mplx, rst_unprocessed_stream, session);
}
+/* APR callback invoked if allocation fails. */
+static int abort_on_oom(int retcode)
+{
+ ap_abort_on_oom();
+ return retcode; /* unreachable, hopefully. */
+}
+
static h2_stream *h2_session_open_stream(h2_session *session, int stream_id,
int initiated_on)
{
h2_stream * stream;
+ apr_allocator_t *allocator;
apr_pool_t *stream_pool;
+ apr_status_t rv;
- apr_pool_create(&stream_pool, session->pool);
+ rv = apr_allocator_create(&allocator);
+ if (rv != APR_SUCCESS)
+ return NULL;
+
+ apr_allocator_max_free_set(allocator, ap_max_mem_free);
+ apr_pool_create_ex(&stream_pool, session->pool, NULL, allocator);
+ apr_allocator_owner_set(allocator, stream_pool);
+ apr_pool_abort_set(abort_on_oom, stream_pool);
apr_pool_tag(stream_pool, "h2_stream");
stream = h2_stream_create(stream_id, stream_pool, session,
@@ -972,6 +988,14 @@ apr_status_t h2_session_create(h2_session **psession, conn_rec *c, request_rec *
}
h2_c1_io_init(&session->io, session);
+ /* setup request header scratch buffers */
+ session->hd_scratch.max_len = session->s->limit_req_fieldsize?
+ session->s->limit_req_fieldsize : 8190;
+ session->hd_scratch.name =
+ apr_pcalloc(session->pool, session->hd_scratch.max_len + 1);
+ session->hd_scratch.value =
+ apr_pcalloc(session->pool, session->hd_scratch.max_len + 1);
+
session->padding_max = h2_config_sgeti(s, H2_CONF_PADDING_BITS);
if (session->padding_max) {
session->padding_max = (0x01 << session->padding_max) - 1;
@@ -1032,7 +1056,7 @@ apr_status_t h2_session_create(h2_session **psession, conn_rec *c, request_rec *
n = h2_config_sgeti(s, H2_CONF_PUSH_DIARY_SIZE);
session->push_diary = h2_push_diary_create(session->pool, n);
-
+
if (APLOGcdebug(c)) {
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
H2_SSSN_LOG(APLOGNO(03200), session,
@@ -1699,9 +1723,10 @@ static void on_stream_state_enter(void *ctx, h2_stream *stream)
break;
case H2_SS_CLEANUP:
nghttp2_session_set_stream_user_data(session->ngh2, stream->id, NULL);
+ update_child_status(session, SERVER_BUSY_WRITE, "done", stream);
h2_mplx_c1_stream_cleanup(session->mplx, stream, &session->open_streams);
+ stream = NULL;
++session->streams_done;
- update_child_status(session, SERVER_BUSY_WRITE, "done", stream);
break;
default:
break;
diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h
index 2c8f334cce0..7932a9e2ccf 100644
--- a/modules/http2/h2_session.h
+++ b/modules/http2/h2_session.h
@@ -29,6 +29,7 @@
*/
#include "h2.h"
+#include "h2_util.h"
struct apr_thread_mutext_t;
struct apr_thread_cond_t;
@@ -118,6 +119,8 @@ typedef struct h2_session {
struct h2_iqueue *out_c1_blocked; /* all streams with output blocked on c1 buffer full */
struct h2_iqueue *ready_to_process; /* all streams ready for processing */
+ h2_hd_scratch hd_scratch;
+
} h2_session;
const char *h2_session_state_str(h2_session_state state);
diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c
index 35b53860c03..f8214019404 100644
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -659,7 +659,8 @@ apr_status_t h2_stream_set_request_rec(h2_stream *stream,
if (stream->rst_error) {
return APR_ECONNRESET;
}
- status = h2_request_rcreate(&req, stream->pool, r);
+ status = h2_request_rcreate(&req, stream->pool, r,
+ &stream->session->hd_scratch);
if (status == APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r,
H2_STRM_LOG(APLOGNO(03058), stream,
@@ -691,13 +692,11 @@ static void set_error_response(h2_stream *stream, int http_status)
static apr_status_t add_trailer(h2_stream *stream,
const char *name, size_t nlen,
const char *value, size_t vlen,
- size_t max_field_len, int *pwas_added)
+ h2_hd_scratch *scratch)
{
conn_rec *c = stream->session->c1;
- char *hname, *hvalue;
const char *existing;
- *pwas_added = 0;
if (nlen == 0 || name[0] == ':') {
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_EINVAL, c,
H2_STRM_LOG(APLOGNO(03060), stream,
@@ -710,20 +709,35 @@ static apr_status_t add_trailer(h2_stream *stream,
if (!stream->trailers_in) {
stream->trailers_in = apr_table_make(stream->pool, 5);
}
- hname = apr_pstrndup(stream->pool, name, nlen);
- h2_util_camel_case_header(hname, nlen);
- existing = apr_table_get(stream->trailers_in, hname);
- if (max_field_len
- && ((existing? strlen(existing)+2 : 0) + vlen + nlen + 2 > max_field_len)) {
- /* "key: (oldval, )?nval" is too long */
+
+ if (((nlen + vlen + 2) > scratch->max_len))
return APR_EINVAL;
+
+ /* We need 0-terminated strings to operate on apr_table */
+ AP_DEBUG_ASSERT(nlen < scratch->max_len);
+ memcpy(scratch->name, name, nlen);
+ scratch->name[nlen] = 0;
+ AP_DEBUG_ASSERT(vlen < scratch->max_len);
+ memcpy(scratch->value, value, vlen);
+ scratch->value[vlen] = 0;
+
+ existing = apr_table_get(stream->trailers_in, scratch->name);
+ if(existing) {
+ if (!vlen) /* not adding a 0-length value to existing */
+ return APR_SUCCESS;
+ if ((strlen(existing) + 2 + vlen + nlen + 2 > scratch->max_len)) {
+ /* "name: existing, value" is too long */
+ return APR_EINVAL;
+ }
+ apr_table_merge(stream->trailers_in, scratch->name, scratch->value);
}
- if (!existing) *pwas_added = 1;
- hvalue = apr_pstrndup(stream->pool, value, vlen);
- apr_table_mergen(stream->trailers_in, hname, hvalue);
- ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
- H2_STRM_MSG(stream, "added trailer '%s: %s'"), hname, hvalue);
-
+ else {
+ h2_util_camel_case_header(scratch->name, nlen);
+ apr_table_set(stream->trailers_in, scratch->name, scratch->value);
+ }
+ ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
+ H2_STRM_MSG(stream, "added trailer '%s: %s'"),
+ scratch->name, scratch->value);
return APR_SUCCESS;
}
@@ -732,7 +746,7 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
const char *value, size_t vlen)
{
h2_session *session = stream->session;
- int error = 0, was_added = 0;
+ int error = 0;
apr_status_t status = APR_SUCCESS;
H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
@@ -760,6 +774,7 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
++stream->request_headers_added;
}
else if (H2_SS_IDLE == stream->state) {
+ int was_added;
if (!stream->rtmp) {
if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
++stream->session->remote.emitted_count;
@@ -771,7 +786,7 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
}
status = h2_request_add_header(stream->rtmp, stream->pool,
name, nlen, value, vlen,
- session->s->limit_req_fieldsize, &was_added);
+ &session->hd_scratch, &was_added);
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, session->c1,
H2_STRM_MSG(stream, "add_header: '%.*s: %.*s"),
(int)nlen, name, (int)vlen, value);
@@ -779,8 +794,8 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
}
else if (H2_SS_OPEN == stream->state) {
status = add_trailer(stream, name, nlen, value, vlen,
- session->s->limit_req_fieldsize, &was_added);
- if (was_added) ++stream->request_headers_added;
+ &session->hd_scratch);
+ if (!status) ++stream->request_headers_added;
}
else {
status = APR_EINVAL;
@@ -789,16 +804,17 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
if (APR_EINVAL == status) {
/* header too long */
- if (!h2_stream_is_ready(stream)) {
+ if (!h2_stream_is_ready(stream) && !stream->request_headers_failed) {
ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, session->c1,
- H2_STRM_LOG(APLOGNO(10180), stream,"Request header exceeds "
- "LimitRequestFieldSize: %.*s"),
+ H2_STRM_LOG(APLOGNO(10180), stream,
+ "Request header exceeds LimitRequestFieldSize(%d): %.*s"),
+ (int)session->hd_scratch.max_len,
(int)H2MIN(nlen, 80), name);
}
error = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
goto cleanup;
}
-
+
if (session->s->limit_req_fields > 0
&& stream->request_headers_added > session->s->limit_req_fields) {
/* too many header lines */
@@ -810,12 +826,13 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
if (!h2_stream_is_ready(stream)) {
ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, session->c1,
H2_STRM_LOG(APLOGNO(10181), stream, "Number of request headers "
- "exceeds LimitRequestFields"));
+ "exceeds LimitRequestFields(%d)"),
+ (int)session->s->limit_req_fields);
}
error = HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE;
goto cleanup;
}
-
+
cleanup:
if (error) {
++stream->request_headers_failed;
diff --git a/modules/http2/h2_util.c b/modules/http2/h2_util.c
index 8e53cebdf92..605c348ca12 100644
--- a/modules/http2/h2_util.c
+++ b/modules/http2/h2_util.c
@@ -1693,10 +1693,9 @@ int h2_ignore_resp_trailer(const char *name, size_t len)
}
static apr_status_t req_add_header(apr_table_t *headers, apr_pool_t *pool,
- nghttp2_nv *nv, size_t max_field_len,
+ nghttp2_nv *nv, h2_hd_scratch *scratch,
int *pwas_added)
{
- char *hname, *hvalue;
const char *existing;
*pwas_added = 0;
@@ -1712,15 +1711,14 @@ static apr_status_t req_add_header(apr_table_t *headers, apr_pool_t *pool,
/* Cookie header come separately in HTTP/2, but need
* to be merged by "; " (instead of default ", ")
*/
- if (max_field_len
- && strlen(existing) + nv->valuelen + nv->namelen + 4
- > max_field_len) {
+ if ((strlen(existing) + nv->valuelen + nv->namelen + 4)
+ > scratch->max_len) {
/* "key: oldval, nval" is too long */
return APR_EINVAL;
}
- hvalue = apr_pstrndup(pool, (const char*)nv->value, nv->valuelen);
apr_table_setn(headers, "Cookie",
- apr_psprintf(pool, "%s; %s", existing, hvalue));
+ apr_psprintf(pool, "%s; %.*s", existing,
+ (int)nv->valuelen, nv->value));
return APR_SUCCESS;
}
}
@@ -1731,27 +1729,40 @@ static apr_status_t req_add_header(apr_table_t *headers, apr_pool_t *pool,
}
}
- hname = apr_pstrndup(pool, (const char*)nv->name, nv->namelen);
- h2_util_camel_case_header(hname, nv->namelen);
- existing = apr_table_get(headers, hname);
- if (max_field_len) {
- if ((existing? strlen(existing)+2 : 0) + nv->valuelen + nv->namelen + 2
- > max_field_len) {
- /* "key: (oldval, )?nval" is too long */
+ if (((nv->namelen + nv->valuelen + 2) > scratch->max_len))
+ return APR_EINVAL;
+
+ /* We need 0-terminated strings to operate on apr_table */
+ AP_DEBUG_ASSERT(nv->namelen < scratch->max_len);
+ memcpy(scratch->name, nv->name, nv->namelen);
+ scratch->name[nv->namelen] = 0;
+ AP_DEBUG_ASSERT(nv->valuelen < scratch->max_len);
+ memcpy(scratch->value, nv->value, nv->valuelen);
+ scratch->value[nv->valuelen] = 0;
+
+ *pwas_added = 1;
+ existing = apr_table_get(headers, scratch->name);
+ if (existing) {
+ if (!nv->valuelen) /* not adding a 0-length value to existing */
+ return APR_SUCCESS;
+ if ((strlen(existing) + 2 + nv->valuelen + nv->namelen + 2)
+ > scratch->max_len) {
+ /* "name: existing, value" is too long */
return APR_EINVAL;
}
+ apr_table_merge(headers, scratch->name, scratch->value);
+ }
+ else {
+ h2_util_camel_case_header(scratch->name, nv->namelen);
+ apr_table_set(headers, scratch->name, scratch->value);
}
- if (!existing) *pwas_added = 1;
- hvalue = apr_pstrndup(pool, (const char*)nv->value, nv->valuelen);
- apr_table_mergen(headers, hname, hvalue);
-
return APR_SUCCESS;
}
apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool,
const char *name, size_t nlen,
const char *value, size_t vlen,
- size_t max_field_len, int *pwas_added)
+ h2_hd_scratch *scratch, int *pwas_added)
{
nghttp2_nv nv;
@@ -1759,7 +1770,7 @@ apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool,
nv.namelen = nlen;
nv.value = (uint8_t*)value;
nv.valuelen = vlen;
- return req_add_header(headers, pool, &nv, max_field_len, pwas_added);
+ return req_add_header(headers, pool, &nv, scratch, pwas_added);
}
/*******************************************************************************
diff --git a/modules/http2/h2_util.h b/modules/http2/h2_util.h
index d2e6548ba87..c2cab4afa45 100644
--- a/modules/http2/h2_util.h
+++ b/modules/http2/h2_util.h
@@ -397,14 +397,21 @@ apr_status_t h2_req_create_ngheader(h2_ngheader **ph, apr_pool_t *p,
const struct h2_request *req);
#endif
+typedef struct h2_hd_scratch {
+ size_t max_len; /* header field size name + ': ' + value */
+ char *name; /* max_len+1 sized */
+ char *value; /* max_len+1 sized */
+
+} h2_hd_scratch;
+
/**
* Add a HTTP/2 header and return the table key if it really was added
* and not ignored.
*/
-apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool,
+apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool,
const char *name, size_t nlen,
const char *value, size_t vlen,
- size_t max_field_len, int *pwas_added);
+ h2_hd_scratch *scratch, int *pwas_added);
/*******************************************************************************
* apr brigade helpers
diff --git a/test/modules/http2/test_200_header_invalid.py b/test/modules/http2/test_200_header_invalid.py
index 6b73301c282..1687e3d9818 100644
--- a/test/modules/http2/test_200_header_invalid.py
+++ b/test/modules/http2/test_200_header_invalid.py
@@ -133,7 +133,7 @@ def test_h2_200_11(self, env):
assert 431 == r.response["status"]
# test header field count, LimitRequestFields (default 100)
- # see #201: several headers with same name are mered and count only once
+ # see #201: several headers with same name are merged and counted
def test_h2_200_12(self, env):
url = env.mkurl("https", "cgi", "/")
opt = []
@@ -143,7 +143,7 @@ def test_h2_200_12(self, env):
r = env.curl_get(url, options=opt)
assert r.response["status"] == 200
r = env.curl_get(url, options=(opt + ["-H", "y: 2"]))
- assert r.response["status"] == 200
+ assert r.response["status"] == 431
# test header field count, LimitRequestFields (default 100)
# different header names count each
Reference in New Issue View Git Blame Copy Permalink