From b1c327d6d8105cd6dc8743d43c082bd8c994323e Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Fri, 17 Jul 2026 11:08:21 +0100 Subject: [PATCH 01/13] mod_substitute: fix heap over-read in set_pattern() delimiter scanning * modules/filters/mod_substitute.c (set_pattern): Guard the pre-incrementing delimiter scan loops with a NUL check, preventing a read past the end of the allocation when the from or to field has no closing delimiter. Assisted-by: Claude Sonnet 4.6 (cherry picked from commit d1e04e6ffdc3d08612575d25380724b7d5c5c433) --- changes-entries/substitute-pattern-oob-read.txt | 2 ++ modules/filters/mod_substitute.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 changes-entries/substitute-pattern-oob-read.txt diff --git a/changes-entries/substitute-pattern-oob-read.txt b/changes-entries/substitute-pattern-oob-read.txt new file mode 100644 index 00000000000..55eea6f041b --- /dev/null +++ b/changes-entries/substitute-pattern-oob-read.txt @@ -0,0 +1,2 @@ + *) mod_substitute: Fix crash or misbehaviour when loading a Substitute + directive with a missing closing delimiter. [Joe Orton] diff --git a/modules/filters/mod_substitute.c b/modules/filters/mod_substitute.c index d454bf347cf..474dc41b8c6 100644 --- a/modules/filters/mod_substitute.c +++ b/modules/filters/mod_substitute.c @@ -627,7 +627,7 @@ static const char *set_pattern(cmd_parms *cmd, void *cfg, const char *line) if (delim) from = ++ourline; if (from) { - if (*ourline != delim) { + if (*ourline && *ourline != delim) { while (*++ourline && *ourline != delim); } if (*ourline) { @@ -636,7 +636,7 @@ static const char *set_pattern(cmd_parms *cmd, void *cfg, const char *line) } } if (to) { - if (*ourline != delim) { + if (*ourline && *ourline != delim) { while (*++ourline && *ourline != delim); } if (*ourline) { From 1a5e11309f4fa2a0b48dfb607135f3b875bd5dd7 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Fri, 17 Jul 2026 11:08:02 +0100 Subject: [PATCH 02/13] mod_substitute: reject overflow values in SubstituteMaxLineLength * modules/filters/mod_substitute.c (set_max_line_length): Check that the parsed value does not exceed APR_INT64_MAX / multiplier before applying the K/M/G suffix, to avoid signed integer overflow UB. Assisted-by: Claude Sonnet 4.6 (cherry picked from commit 62c9f8fc6d0721501eaec81bd0ea0f16643f1ebf) --- changes-entries/substitute-maxlinelength-overflow.txt | 2 ++ modules/filters/mod_substitute.c | 6 ++++++ 2 files changed, 8 insertions(+) create mode 100644 changes-entries/substitute-maxlinelength-overflow.txt diff --git a/changes-entries/substitute-maxlinelength-overflow.txt b/changes-entries/substitute-maxlinelength-overflow.txt new file mode 100644 index 00000000000..7ad53978121 --- /dev/null +++ b/changes-entries/substitute-maxlinelength-overflow.txt @@ -0,0 +1,2 @@ + *) mod_substitute: Fix SubstituteMaxLineLength to reject values too + large for the K/M/G suffix. [Joe Orton] diff --git a/modules/filters/mod_substitute.c b/modules/filters/mod_substitute.c index 474dc41b8c6..0b3dc806a4a 100644 --- a/modules/filters/mod_substitute.c +++ b/modules/filters/mod_substitute.c @@ -713,12 +713,18 @@ static const char *set_max_line_length(cmd_parms *cmd, void *cfg, const char *ar rv = apr_strtoff(&max, arg, &end, 10); if (rv == APR_SUCCESS) { if ((*end == 'K' || *end == 'k') && !end[1]) { + if (max > APR_INT64_MAX / KBYTE) + return "SubstituteMaxLineLength value too large"; max *= KBYTE; } else if ((*end == 'M' || *end == 'm') && !end[1]) { + if (max > APR_INT64_MAX / MBYTE) + return "SubstituteMaxLineLength value too large"; max *= MBYTE; } else if ((*end == 'G' || *end == 'g') && !end[1]) { + if (max > APR_INT64_MAX / GBYTE) + return "SubstituteMaxLineLength value too large"; max *= GBYTE; } else if (*end && /* neither empty nor [Bb] */ From 030bb47c66440d6e127d85b047eaafc65651ebf4 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Fri, 17 Jul 2026 11:03:42 +0100 Subject: [PATCH 03/13] mod_ssl: fix set_challenge_creds() to return rv on failure * modules/ssl/ssl_engine_kernel.c (set_challenge_creds): Return rv rather than APR_SUCCESS unconditionally, so credential setup failures are propagated to the ALPN selection callback. Assisted-by: Claude Sonnet 4.6 (cherry picked from commit b85e02461be91694337b6b4a9d39f2d447053f23) --- modules/ssl/ssl_engine_kernel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c index 88ff5d19023..92458526c70 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/modules/ssl/ssl_engine_kernel.c @@ -2195,7 +2195,7 @@ static apr_status_t set_challenge_creds(conn_rec *c, const char *servername, cleanup: if (our_data && cert) X509_free(cert); if (our_data && key) EVP_PKEY_free(key); - return APR_SUCCESS; + return rv; } /* From 8465655a43613882536baa59ce2df0851f5c14b8 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Fri, 17 Jul 2026 08:40:21 +0100 Subject: [PATCH 04/13] mod_cern_meta: reject HTTP framing headers in metadata files * modules/metadata/mod_cern_meta.c (scan_meta_file): Return a 500 error if a framing header is found in a .meta file rather than merging it into the response headers. Assisted-by: Claude Sonnet 4.6 (cherry picked from commit 4d8d05143f40191c7cf14076a85d947343a61329) --- changes-entries/cern-meta-header-injection.txt | 2 ++ modules/metadata/mod_cern_meta.c | 12 ++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 changes-entries/cern-meta-header-injection.txt diff --git a/changes-entries/cern-meta-header-injection.txt b/changes-entries/cern-meta-header-injection.txt new file mode 100644 index 00000000000..2aef1eec926 --- /dev/null +++ b/changes-entries/cern-meta-header-injection.txt @@ -0,0 +1,2 @@ + *) mod_cern_meta: Reject HTTP framing headers in metadata files to prevent + response splitting. [Joe Orton] diff --git a/modules/metadata/mod_cern_meta.c b/modules/metadata/mod_cern_meta.c index 3f36b2dba8a..a150b3c9fa1 100644 --- a/modules/metadata/mod_cern_meta.c +++ b/modules/metadata/mod_cern_meta.c @@ -256,6 +256,18 @@ static int scan_meta_file(request_rec *r, apr_file_t *f) sscanf(l, "%d", &r->status); r->status_line = apr_pstrdup(r->pool, l); } + else if (!ap_cstr_casecmp(w, "Transfer-Encoding") + || !ap_cstr_casecmp(w, "Content-Length") + || !ap_cstr_casecmp(w, "Connection") + || !ap_cstr_casecmp(w, "Trailer") + || !ap_cstr_casecmp(w, "Upgrade") + || !ap_cstr_casecmp(w, "Keep-Alive") + || !ap_cstr_casecmp(w, "TE")) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10596) + "forbidden HTTP framing header '%s' in meta file: %s", + w, r->filename); + return HTTP_INTERNAL_SERVER_ERROR; + } else { apr_table_set(tmp_headers, w, l); } From 032a04af1ee9aeb018479df14bb7e9ad10b2ad2b Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Fri, 17 Jul 2026 08:39:16 +0100 Subject: [PATCH 05/13] mod_ssl: fix NULL dereference in OCSP responder URI parsing * modules/ssl/ssl_engine_ocsp.c (determine_responder_uri): Check u->scheme is non-NULL before calling ap_cstr_casecmp(), since apr_uri_parse() can succeed with a NULL scheme for scheme-less URIs. Assisted-by: Claude Sonnet 4.6 (cherry picked from commit ac2deae4b60ce0dbc9f1d85363d482e5c370901f) --- modules/ssl/ssl_engine_ocsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/ssl/ssl_engine_ocsp.c b/modules/ssl/ssl_engine_ocsp.c index 539ed103eae..6a03fb41744 100644 --- a/modules/ssl/ssl_engine_ocsp.c +++ b/modules/ssl/ssl_engine_ocsp.c @@ -80,7 +80,7 @@ static apr_uri_t *determine_responder_uri(SSLSrvConfigRec *sc, X509 *cert, } rv = apr_uri_parse(p, s, u); - if (rv || !u->hostname) { + if (rv || !u->hostname || !u->scheme) { ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, c, APLOGNO(01919) "failed to parse OCSP responder URI '%s'", s); return NULL; From 3075f4440e22a55c4af7b3739c854e9fd6f51699 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 6 Jul 2026 12:08:49 +0000 Subject: [PATCH 06/13] * modules/ssl/ssl_util.c (ssl_asn1_table_set): Add assertion for the (likely unreachable) i2d_PrivateKey() failure case. Submitted by: Sayed Kaif Github: closes #619 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1935941 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit 4f8722996978bf8e74bb206a3b13d189a2792a0a) --- modules/ssl/ssl_util.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/ssl/ssl_util.c b/modules/ssl/ssl_util.c index 7473edbe6c0..5e49f009cdf 100644 --- a/modules/ssl/ssl_util.c +++ b/modules/ssl/ssl_util.c @@ -201,9 +201,13 @@ ssl_asn1_t *ssl_asn1_table_set(apr_hash_t *table, const char *key, { apr_ssize_t klen = strlen(key); ssl_asn1_t *asn1 = apr_hash_get(table, key, klen); - apr_size_t length = i2d_PrivateKey(pkey, NULL); + int derlen = i2d_PrivateKey(pkey, NULL); + apr_size_t length; unsigned char *p; + ap_assert(derlen > 0); /* should never happen for any loaded key */ + length = (apr_size_t)derlen; + /* Re-use structure if cached previously. */ if (asn1) { if (asn1->nData != length) { From 3de54611ed1e40430d4c35f0d37ba2b4d4154413 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 6 Jul 2026 11:51:31 +0000 Subject: [PATCH 07/13] * modules/proxy/balancers/mod_lbmethod_heartbeat.c (hb_parse_int): New helper replacing atoi() with safe integer parsing via apr_strtoi64, with range validation. (readfile_heartbeats): Use hb_parse_int for busy, ready, lastseen, and port fields. (find_best_hb): Add overflow-safe saturation arithmetic for openslots accumulation and the pick loop upper bound. Submitted by: Sayed Kaif Github: closes #629 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1935935 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit feb1c6ebbd8ab43386526c112aeb9eea7c9a6fa7) --- .../proxy/balancers/mod_lbmethod_heartbeat.c | 62 ++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/modules/proxy/balancers/mod_lbmethod_heartbeat.c b/modules/proxy/balancers/mod_lbmethod_heartbeat.c index 0534e5b96ab..16829973fbe 100644 --- a/modules/proxy/balancers/mod_lbmethod_heartbeat.c +++ b/modules/proxy/balancers/mod_lbmethod_heartbeat.c @@ -61,6 +61,28 @@ typedef struct ctx_servers { apr_hash_t *servers; } ctx_servers_t; +static int hb_parse_int(const char *val, int min, int max, int *result) +{ + apr_int64_t parsed; + char *end = NULL; + + if (!val || !*val) { + return 0; + } + + errno = 0; + parsed = apr_strtoi64(val, &end, 10); + if (errno == ERANGE || end == val || *end != '\0') { + return 0; + } + if (parsed < min || parsed > max) { + return 0; + } + + *result = (int)parsed; + return 1; +} + static void argstr_to_table(apr_pool_t *p, char *str, apr_table_t *parms) { @@ -179,19 +201,31 @@ static apr_status_t readfile_heartbeats(const char *path, apr_hash_t *servers, argstr_to_table(pool, apr_pstrdup(pool, t), hbt); if ((val = apr_table_get(hbt, "busy"))) { - server->busy = atoi(val); + int parsed; + if (hb_parse_int(val, 0, INT_MAX, &parsed)) { + server->busy = parsed; + } } if ((val = apr_table_get(hbt, "ready"))) { - server->ready = atoi(val); + int parsed; + if (hb_parse_int(val, 0, INT_MAX, &parsed)) { + server->ready = parsed; + } } if ((val = apr_table_get(hbt, "lastseen"))) { - server->seen = atoi(val); + int parsed; + if (hb_parse_int(val, 0, INT_MAX, &parsed)) { + server->seen = parsed; + } } if ((val = apr_table_get(hbt, "port"))) { - server->port = atoi(val); + int parsed; + if (hb_parse_int(val, 1, 65535, &parsed)) { + server->port = parsed; + } } if (server->busy == 0 && server->ready != 0) { @@ -312,7 +346,13 @@ static proxy_worker *find_best_hb(proxy_balancer *balancer, if (PROXY_WORKER_IS_USABLE(*worker)) { server->worker = *worker; if (server->seen < LBM_HEARTBEAT_MAX_LASTSEEN) { - openslots += server->ready; + apr_uint32_t ready = (apr_uint32_t)server->ready; + if (ready > APR_UINT32_MAX - openslots) { + openslots = APR_UINT32_MAX; + } + else { + openslots += ready; + } APR_ARRAY_PUSH(up_servers, hb_server_t *) = server; } } @@ -325,12 +365,20 @@ static proxy_worker *find_best_hb(proxy_balancer *balancer, pick = ap_random_pick(0, openslots); for (i = 0; i < up_servers->nelts; i++) { + apr_uint32_t upper; server = APR_ARRAY_IDX(up_servers, i, hb_server_t *); - if (pick >= c && pick <= c + server->ready) { + if ((apr_uint32_t)server->ready > APR_UINT32_MAX - c) { + upper = APR_UINT32_MAX; + } + else { + upper = c + (apr_uint32_t)server->ready; + } + + if (pick >= c && pick <= upper) { mycandidate = server->worker; } - c += server->ready; + c = upper; } } From d28c4a85048e9bf506b3d9ba69eece5fc5747269 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Thu, 11 Jun 2026 11:38:41 +0000 Subject: [PATCH 08/13] * modules/generators/mod_cgid.c (close_unix_socket): Return errno on failure rather than -1. (sock_write): Handle short writes. (cgid_init): Fix off-by-one in socket path truncation. Assisted-by: Claude Opus 4.6 GitHub: resolves PR#669 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1935193 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit 8e210c28a1dfd84a1e9c4fb8fd3d646619639dfd) --- modules/generators/mod_cgid.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c index a0ef2b51699..452b97ac9d5 100644 --- a/modules/generators/mod_cgid.c +++ b/modules/generators/mod_cgid.c @@ -342,7 +342,7 @@ static apr_status_t close_unix_socket(void *thefd) { int fd = (int)((long)thefd); - return close(fd); + return close(fd) < 0 ? errno : APR_SUCCESS; } /* Read from the socket dealing with incomplete messages and signals. @@ -431,13 +431,18 @@ static apr_status_t sock_read(int fd, void *vbuf, size_t buf_size) static apr_status_t sock_write(int fd, const void *buf, size_t buf_size) { int rc; + const char *b = buf; + size_t written = 0; do { - rc = write(fd, buf, buf_size); - } while (rc < 0 && errno == EINTR); - if (rc < 0) { - return errno; - } + do { + rc = write(fd, b + written, buf_size - written); + } while (rc < 0 && errno == EINTR); + if (rc < 0) { + return errno; + } + written += rc; + } while (written < buf_size); return APR_SUCCESS; } @@ -1062,7 +1067,7 @@ static int cgid_init(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp, parent_pid = getpid(); tmp_sockname = ap_runtime_dir_relative(p, sockname); if (strlen(tmp_sockname) > sizeof(server_addr->sun_path) - 1) { - tmp_sockname[sizeof(server_addr->sun_path)] = '\0'; + tmp_sockname[sizeof(server_addr->sun_path) - 1] = '\0'; ap_log_error(APLOG_MARK, APLOG_ERR, 0, main_server, APLOGNO(01254) "The length of the ScriptSock path exceeds maximum, " "truncating to %s", tmp_sockname); From 2dce6c4f64f8979ccbea0c207dd09dff054e3fea Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Thu, 11 Jun 2026 11:38:22 +0000 Subject: [PATCH 09/13] * modules/generators/mod_cgid.c (get_req): Fix wrong sizeof in allocation of core_request_config, which used sizeof(core_module). (cgid_server): Fix stale rv passed to ap_log_error for passed fd debug message. (include_cmd): Fix double registration of cleanup_script which could kill a garbage pid when get_cgi_pid failed. Check return value of send_req. Change return type to apr_status_t to match declaration in cgi_common.h Assisted-by: Claude Opus 4.6 GitHub: PR#669 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1935192 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit 489f5ef688fedb0139b4428bc04d0e18493e430f) --- modules/generators/mod_cgid.c | 37 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c index 452b97ac9d5..52bbb4f9a18 100644 --- a/modules/generators/mod_cgid.c +++ b/modules/generators/mod_cgid.c @@ -535,7 +535,7 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, /* handle module indexes and such */ rconf = (void **)ap_create_request_config(r->pool); - temp_core = (core_request_config *)apr_palloc(r->pool, sizeof(core_module)); + temp_core = (core_request_config *)apr_palloc(r->pool, sizeof *temp_core); rconf[AP_CORE_MODULE_INDEX] = (void *)temp_core; r->request_config = (ap_conf_vector_t *)rconf; ap_set_module_config(r->request_config, &cgid_module, (void *)&req->ugid); @@ -867,7 +867,7 @@ static int cgid_server(void *data) errfileno = STDERR_FILENO; } else { - ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, main_server, + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, main_server, "using passed fd %d as stderr", errfileno); /* Limit the received fd lifetime to pool lifetime */ apr_pool_cleanup_register(ptrans, (void *)((long)errfileno), @@ -1728,8 +1728,8 @@ static void add_ssi_vars(request_rec *r) } } -static int include_cmd(include_ctx_t *ctx, ap_filter_t *f, - apr_bucket_brigade *bb, const char *command) +static apr_status_t include_cmd(include_ctx_t *ctx, ap_filter_t *f, + apr_bucket_brigade *bb, const char *command) { char **env; int sd; @@ -1747,30 +1747,29 @@ static int include_cmd(include_ctx_t *ctx, ap_filter_t *f, env = ap_create_environment(r->pool, r->subprocess_env); if ((retval = connect_to_daemon(&sd, r, conf)) != OK) { - return retval; + return APR_EGENERAL; } - send_req(sd, NULL, r, command, env, SSI_REQ); + rv = send_req(sd, NULL, r, command, env, SSI_REQ); + if (rv) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, + "could not send request to cgi daemon (for SSI)"); + return rv; + } info = apr_palloc(r->pool, sizeof(struct cleanup_script_info)); info->conf = conf; info->r = r; rv = get_cgi_pid(r, conf, &(info->pid)); - if (APR_SUCCESS == rv) { - /* for this type of request, the script is invoked through an - * intermediate shell process... cleanup_script is only able - * to knock out the shell process, not the actual script - */ - apr_pool_cleanup_register(r->pool, info, - cleanup_script, - apr_pool_cleanup_null); - } - else { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, "error determining cgi PID (for SSI)"); + if (rv) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, "error determining cgi daemon PID (for SSI)"); + return rv; } - apr_pool_cleanup_register(r->pool, info, - cleanup_script, + /* For this type of request, the script is invoked through an + * intermediate shell process... cleanup_script is only able to + * knock out the shell process, not the actual script. */ + apr_pool_cleanup_register(r->pool, info, cleanup_script, apr_pool_cleanup_null); /* We are putting the socket discriptor into an apr_file_t so that we can From c0b3af94447e7d2d87fe72536623308fd67ef845 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Thu, 11 Jun 2026 11:37:45 +0000 Subject: [PATCH 10/13] * modules/generators/mod_cgid.c (cgid_req_t): Change env_count to unsigned. Define ENV_COUNT_MAX. (get_req): Add upper bounds for uri_len, args_len, and env_count. Validate per-variable length in environment reading loop. Move validation before use of loglevel. Assisted-by: Claude Opus 4.6 GitHub: PR#669 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1935191 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit d7ac43a29f73ed60dcb322abad41a93a10394784) --- modules/generators/mod_cgid.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c index 52bbb4f9a18..737d9c92d4e 100644 --- a/modules/generators/mod_cgid.c +++ b/modules/generators/mod_cgid.c @@ -192,6 +192,8 @@ typedef struct { } cgid_rlimit_t; #endif +#define ENV_COUNT_MAX (256) + typedef struct { int req_type; /* request type (CGI_REQ, SSI_REQ, etc.) */ unsigned long conn_id; /* connection id; daemon uses this as a hash value @@ -201,7 +203,7 @@ typedef struct { pid_t ppid; /* sanity check for config problems leading to * wrong cgid socket use */ - int env_count; + unsigned env_count; ap_unix_identity_t ugid; apr_size_t filename_len; apr_size_t argv0_len; @@ -518,6 +520,11 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, if (stat != APR_SUCCESS) { return stat; } + + if (req->loglevel > APLOG_TRACE8) { + return APR_EINVAL; + } + r->server->log.level = req->loglevel; if (req->req_type == GETPID_REQ) { /* no more data sent for this request */ @@ -525,13 +532,14 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, } /* Sanity check the structure received. */ - if (req->env_count < 0 || req->uri_len == 0 - || req->filename_len > APR_PATH_MAX || req->filename_len == 0 - || req->argv0_len > APR_PATH_MAX || req->argv0_len == 0 - || req->loglevel > APLOG_TRACE8) { + if (req->env_count > ENV_COUNT_MAX + || req->filename_len == 0 || req->filename_len > APR_PATH_MAX + || req->argv0_len == 0 || req->argv0_len > APR_PATH_MAX + || req->uri_len == 0 || req->uri_len > APR_PATH_MAX + || req->args_len > APR_PATH_MAX) { return APR_EINVAL; } - + /* handle module indexes and such */ rconf = (void **)ap_create_request_config(r->pool); @@ -565,6 +573,9 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, if ((stat = sock_read(fd, &curlen, sizeof(curlen))) != APR_SUCCESS) { return stat; } + if (curlen > APR_PATH_MAX) { + return APR_EINVAL; + } environ[i] = apr_pcalloc(r->pool, curlen + 1); if ((stat = sock_read(fd, environ[i], curlen)) != APR_SUCCESS) { return stat; From 8b96f5e74fc0a6c1e477ef9483cc6d96018ef2e7 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 6 Jul 2026 08:15:14 +0000 Subject: [PATCH 11/13] * modules/md/md_crypt.c (md_cert_get_ari_cert_id): Don't used fixed buffer size for serial number, fail if ASN.1->BIGNUM conversion fails. Github: closes #680 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1935926 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit 7612f2b72f6529f949d0a099df641f075542f9a3) --- modules/md/md_crypt.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/md/md_crypt.c b/modules/md/md_crypt.c index eef12683539..a90eb0fc9b4 100644 --- a/modules/md/md_crypt.c +++ b/modules/md/md_crypt.c @@ -2226,7 +2226,7 @@ apr_status_t md_cert_get_ari_cert_id(const char **pari_cert_id, const ASN1_INTEGER *serial; BIGNUM *bn; int i = -1, sder_len; - unsigned char *ucp, sbuf[256]; + unsigned char *ucp, *sbuf; *pari_cert_id = NULL; s_aki = X509_get_ext_d2i(cert->x509, NID_authority_key_identifier, &i, NULL); @@ -2253,6 +2253,10 @@ apr_status_t md_cert_get_ari_cert_id(const char **pari_cert_id, } memset(&ser_buf, 0, sizeof(ser_buf)); bn = ASN1_INTEGER_to_BN(serial, NULL); + if (!bn) { + return APR_EINVAL; + } + sbuf = apr_pcalloc(p, BN_num_bytes(bn)); sder_len = BN_bn2bin(bn, sbuf); BN_free(bn); if (sder_len < 1) From eb1557b431d99283ca4d66d7976cd5f554f9e0e6 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 10 Aug 2026 12:49:40 +0000 Subject: [PATCH 12/13] * modules/http/http_filters.c (validate_status_line): Only accept space as the status-code separator, not other whitespace. * server/protocol.c (ap_send_interim_response): Reject control characters in the reason phrase of interim responses. * test/modules/proxy/test_06_interim_resp.py: New test case. Assisted-by: Claude Opus 4.6 GitHub: closes #702 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1937041 13f79535-47bb-0310-9956-ffa450edef68 (cherry picked from commit fbedef2af2feaa99117a901989566b5d2b844f00) --- modules/http/http_filters.c | 2 +- server/protocol.c | 4 +- test/modules/proxy/test_06_interim_resp.py | 182 +++++++++++++++++++++ 3 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 test/modules/proxy/test_06_interim_resp.py diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c index 51b1126aec4..320ff9aef8c 100644 --- a/modules/http/http_filters.c +++ b/modules/http/http_filters.c @@ -1001,7 +1001,7 @@ static apr_status_t validate_status_line(request_rec *r) if (len < 3 || apr_strtoi64(r->status_line, &end, 10) != r->status || (end - 3) != r->status_line - || (len >= 4 && ! apr_isspace(r->status_line[3]))) { + || (len >= 4 && r->status_line[3] != ' ')) { r->status_line = NULL; return APR_EGENERAL; } diff --git a/server/protocol.c b/server/protocol.c index 3a989cfb04f..6fd11793552 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -2349,7 +2349,9 @@ AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers) } status_line = r->status_line; - if (status_line == NULL) { + if (status_line == NULL + || (strlen(status_line) > 4 + && *ap_scan_http_field_content(status_line + 4))) { status_line = ap_get_status_line_ex(r->pool, r->status); } response_line = apr_pstrcat(r->pool, diff --git a/test/modules/proxy/test_06_interim_resp.py b/test/modules/proxy/test_06_interim_resp.py new file mode 100644 index 00000000000..2382649aff4 --- /dev/null +++ b/test/modules/proxy/test_06_interim_resp.py @@ -0,0 +1,182 @@ +import socket +from threading import Thread + +import pytest + +from pyhttpd.conf import HttpdConf +from .env import TCPFaker + + +class _StatusLineBackend(TCPFaker): + """Backend that sends various status line formats.""" + + def __init__(self, host, port, mode="final-status-sep-cr"): + super().__init__(host, port) + self._mode = mode + + def _make_response(self, data): + if self._mode == "final-status-sep-cr": + return ( + b"HTTP/1.1 200\rX-Foobar: abc\r\n" + b"Content-Length: 2\r\n" + b"Content-Type: text/plain\r\n" + b"\r\n" + b"OK" + ) + elif self._mode == "interim-status-cr": + return ( + b"HTTP/1.1 103 Early\rX-Foobar: abc\r\n" + b"X-Early: whatever\r\n" + b"\r\n" + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: 2\r\n" + b"Content-Type: text/plain\r\n" + b"\r\n" + b"OK" + ) + elif self._mode == "interim-102": + return ( + b"HTTP/1.1 102 Processing\r\n" + b"\r\n" + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: 2\r\n" + b"Content-Type: text/plain\r\n" + b"\r\n" + b"OK" + ) + return super()._make_response(data) + + +def _recv_all(sock, timeout=5): + sock.settimeout(timeout) + data = b"" + while True: + try: + chunk = sock.recv(4096) + if not chunk: + break + data += chunk + except socket.timeout: + break + return data + + +class TestStatusLineCR: + """Verify that bare CR in backend status lines is rejected. + + Two cases: + 1. Final response with CR at the separator position (byte 3 of status_line) + 2. Interim 1xx response with CR in the reason phrase + """ + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env): + conf = HttpdConf(env) + conf.start_vhost(domains=[f"test1.{env.http_tld}"], port=env.http_port, + doc_root="htdocs", with_ssl=False) + conf.add([ + f"ProxyPass / http://127.0.0.1:{env.http_port2}/", + f"ProxyPassReverse / http://127.0.0.1:{env.http_port2}/", + ]) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + yield + + def test_proxy_06_001_final_status_sep_cr(self, env): + """A final response with CR at status-code separator must not + forward attacker text as the reason phrase.""" + faker = _StatusLineBackend("127.0.0.1", env.http_port2, + mode="final-status-sep-cr") + faker.start() + try: + with socket.create_connection(('localhost', int(env.http_port))) as sock: + req = ( + f"GET / HTTP/1.0\r\n" + f"Host: test1.{env.http_tld}\r\n" + f"\r\n" + ) + sock.sendall(req.encode()) + sock.shutdown(socket.SHUT_WR) + raw = _recv_all(sock) + finally: + faker.stop() + + status_line = raw.split(b"\r\n")[0] + assert b"\r" not in status_line[:-1] if status_line.endswith(b"\r") else b"\r" not in status_line, \ + f"bare CR in status line: {status_line!r}" + assert b"X-Foobar" not in status_line, \ + f"attacker text in status line: {status_line!r}" + assert b"X-Foobar" not in raw.split(b"\r\n\r\n")[0], \ + f"injected header in response headers: {raw.split(b'\\r\\n\\r\\n')[0]!r}" + + env.httpd_error_log.ignore_recent( + lognos=["AH00957", "AH01106", "AH01114"] + ) + + def test_proxy_06_002_interim_status_cr(self, env): + """An interim 103 response with CR in the reason phrase must not + forward the bare CR to the client.""" + faker = _StatusLineBackend("127.0.0.1", env.http_port2, + mode="interim-status-cr") + faker.start() + try: + with socket.create_connection(('localhost', int(env.http_port))) as sock: + req = ( + f"GET / HTTP/1.1\r\n" + f"Host: test1.{env.http_tld}\r\n" + f"Connection: close\r\n" + f"\r\n" + ) + sock.sendall(req.encode()) + sock.shutdown(socket.SHUT_WR) + raw = _recv_all(sock) + finally: + faker.stop() + + # Split into individual response blocks. The 103 interim response + # comes before the final 200. Check every status line for bare CR. + lines = raw.split(b"\r\n") + for line in lines: + if line.startswith(b"HTTP/"): + assert b"\r" not in line, \ + f"bare CR in status line: {line!r}" + assert b"X-Foobar" not in line, \ + f"attacker text in status line: {line!r}" + + headers_section = raw.split(b"\r\n\r\n")[0] + assert b"X-Foobar" not in headers_section, \ + f"injected header in response: {headers_section!r}" + + env.httpd_error_log.ignore_recent( + lognos=["AH01106"] + ) + + def test_proxy_06_003_interim_102_ok(self, env): + """A well-formed 102 Processing interim response is forwarded + correctly, followed by the final 200.""" + faker = _StatusLineBackend("127.0.0.1", env.http_port2, + mode="interim-102") + faker.start() + try: + with socket.create_connection(('localhost', int(env.http_port))) as sock: + req = ( + f"GET / HTTP/1.1\r\n" + f"Host: test1.{env.http_tld}\r\n" + f"Connection: close\r\n" + f"\r\n" + ) + sock.sendall(req.encode()) + sock.shutdown(socket.SHUT_WR) + raw = _recv_all(sock) + finally: + faker.stop() + + status_lines = [l for l in raw.split(b"\r\n") + if l.startswith(b"HTTP/")] + assert len(status_lines) == 2, \ + f"expected 2 status lines (102 + 200), got {len(status_lines)}: {status_lines!r}" + assert b"102" in status_lines[0], \ + f"first status line should be 102: {status_lines[0]!r}" + assert b"200" in status_lines[1], \ + f"second status line should be 200: {status_lines[1]!r}" From 62b26a37061c7a9f1077a3ebd41a72545882c992 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 10 Aug 2026 15:40:32 +0100 Subject: [PATCH 13/13] Add changes-entries/. --- changes-entries/hardening.txt | 4 ++++ changes-entries/interim-response-reason.txt | 2 ++ 2 files changed, 6 insertions(+) create mode 100644 changes-entries/hardening.txt create mode 100644 changes-entries/interim-response-reason.txt diff --git a/changes-entries/hardening.txt b/changes-entries/hardening.txt new file mode 100644 index 00000000000..df55a52a784 --- /dev/null +++ b/changes-entries/hardening.txt @@ -0,0 +1,4 @@ + *) mod_cgid, mod_ssl, mod_md: Various hardening fixes. [Joe Orton, ] + + *) mod_lbmethod_heartbeat: Use safe integer parsing with range + validation, replacing atoi(). [Sayed Kaif ] diff --git a/changes-entries/interim-response-reason.txt b/changes-entries/interim-response-reason.txt new file mode 100644 index 00000000000..e5a2bbb96e8 --- /dev/null +++ b/changes-entries/interim-response-reason.txt @@ -0,0 +1,2 @@ + *) core: Reject control characters in the reason phrase of interim + responses. Only accept space as status-code separator. [Joe Orton]