diff --git a/CHANGES b/CHANGES index 8ae835854..e662c60de 100644 --- a/CHANGES +++ b/CHANGES @@ -14,6 +14,12 @@ Changes with FreeUnit 1.35.4 xx xxx 2026 *) Bugfix: replace removed cgi module with email.parser in Python upload test fixture; fixes test suite compatibility with Python 3.13. + *) Bugfix: harden HTTP/URI parser and string helper bounds — proxy + response Content-Length underflow guarded in nxt_h1proto.c, oversized + upstream Content-Length values are now logged and flagged inconsistent + in nxt_http_proxy.c, nxt_is_complex_uri_encoded() off-by-one fixed for + trailing "%" sequences, and nxt_rmemstrn() rejects length > buffer. + Changes with FreeUnit 1.35.3 07 Apr 2026 *) Feature: migrate contrib package mirror from packages.nginx.org to diff --git a/src/nxt_h1proto.c b/src/nxt_h1proto.c index dda3900e3..331368faf 100644 --- a/src/nxt_h1proto.c +++ b/src/nxt_h1proto.c @@ -2880,11 +2880,60 @@ nxt_h1p_peer_body_process(nxt_task_t *task, nxt_http_peer_t *peer, } else if (h1p->remainder > 0) { length = nxt_buf_chain_length(out); - h1p->remainder -= length; - if (h1p->remainder == 0) { + /* + * Cast to nxt_off_t can wrap to negative on 64-bit if length + * exceeds NXT_OFF_T_MAX; check that explicitly as well as the + * overrun condition. + */ + if (nxt_slow_path((nxt_off_t) length < 0 + || (nxt_off_t) length > h1p->remainder)) + { + nxt_buf_t *b; + size_t trimmed; + + /* + * Upstream sent more body bytes than its Content-Length + * declared. Truncate the buf chain to remainder bytes so + * we never forward the excess past the Content-Length we + * already advertised downstream, then flag inconsistent + * and close. + */ + nxt_log(task, NXT_LOG_WARN, + "upstream sent %uz body bytes past Content-Length " + "(remainder %O)", length, h1p->remainder); + + trimmed = 0; + for (b = out; b != NULL; b = b->next) { + size_t bsz; + + if (nxt_buf_is_sync(b)) { + continue; + } + bsz = b->mem.free - b->mem.pos; + if (trimmed + bsz <= (size_t) h1p->remainder) { + trimmed += bsz; + continue; + } + /* Trim this buf to fit, drop everything after it. */ + b->mem.free = b->mem.pos + + (size_t) h1p->remainder - trimmed; + b->next = NULL; + break; + } + + peer->request->inconsistent = 1; + h1p->remainder = 0; nxt_buf_chain_add(&out, nxt_http_buf_last(peer->request)); peer->closed = 1; + + } else { + h1p->remainder -= length; + + if (h1p->remainder == 0) { + nxt_buf_chain_add(&out, nxt_http_buf_last(peer->request)); + peer->closed = 1; + } } } diff --git a/src/nxt_http_proxy.c b/src/nxt_http_proxy.c index 7f6ad6866..6ea87dcc1 100644 --- a/src/nxt_http_proxy.c +++ b/src/nxt_http_proxy.c @@ -418,6 +418,20 @@ nxt_http_proxy_content_length(void *ctx, nxt_http_field_t *field, if (nxt_fast_path(n >= 0)) { r->resp.content_length_n = n; + + } else { + /* + * n == -2 means the upstream Content-Length value overflows + * nxt_off_t; n == -1 is a generic parse error. Both are + * inconsistent with a usable response body length, so log and + * mark the response inconsistent rather than silently leaving + * content_length_n at -1. + */ + nxt_log(&r->task, NXT_LOG_WARN, + "upstream Content-Length \"%*s\" is invalid", + (size_t) field->value_length, field->value); + + r->inconsistent = 1; } return NXT_OK; diff --git a/src/nxt_string.c b/src/nxt_string.c index a23ee058f..551ba72b9 100644 --- a/src/nxt_string.c +++ b/src/nxt_string.c @@ -316,6 +316,10 @@ nxt_rmemstrn(const u_char *s, const u_char *end, const char *ss, size_t length) u_char c1, c2; const u_char *s1, *s2; + if (nxt_slow_path(length == 0 || length > (size_t) (end - s))) { + return NULL; + } + s1 = end - length; s2 = (u_char *) ss; c2 = *s2++; @@ -715,7 +719,12 @@ nxt_is_complex_uri_encoded(u_char *src, size_t length) if (nxt_uri_escape[ch >> 5] & (1U << (ch & 0x1f))) { if (ch == '%') { - if (end - src < 2) { + /* + * The two pre-increments below read src+1 and src+2, + * so at least three bytes ('%' plus two hex digits) + * must remain in the buffer. + */ + if (end - src < 3) { return 0; }