[master] Release 2 - #13442
Merged
Merged
[master] Release 2#13442
Conversation
The VLA `char tmp[len * 3 + 1]` is sized from input that ultimately derives from HTTP header values. An attacker sending oversized headers causes a stack allocation of 3x that size, exhausting the stack and crashing the worker. Replace with std::vector<char> which allocates on the heap. This is consistent with the surrounding code which already performs multiple heap allocations to collect and join headers before reaching this function.
Reject len values exceeding INT_MAX to prevent overflow in the len * 3 + 1 buffer calculation and to satisfy the int str_len parameter of TSStringPercentEncode.
Replace std::vector<char> with ts::LocalBuffer<char, 8192> which keeps a stack buffer for common inputs and falls back to heap allocation for larger ones. 8KB covers typical combined header sizes without heap allocation.
* Fix SNI/hostname comparison to check full string length strncasecmp was only comparing up to the length of one string, so names of different lengths could incorrectly match. * Fix typo
* Close connection if unneccesary H2 SETTINGS frame arrives * Address copilot comment
RFC 9113 Section 8.2.2 lists Transfer-Encoding as a connection-specific header that must be rejected as malformed in HTTP/2 messages. The HeaderValidator already rejected Connection, Keep-Alive, Proxy-Connection, and Upgrade but was missing Transfer-Encoding.
* Reject Transfer-Encoding where chunked is not the final coding RFC 9112 requires chunked to be the last transfer coding. Requests that violate this are now rejected with 400, and server responses with 502. Also updates check_hdr_implements to use CSV iteration for multi-value Transfer-Encoding headers. * Address copilot comments * address copilot comments
* Fix out-of-bounds write in MIME obs-fold handling Bounds-check the backward pointer walk in the obs-fold unfold code and use the accumulated line buffer when the preceding bytes are not in the current input fragment. * Address copilot issue
* Enforce per-field size limit in HPACK/QPACK string decoding The existing proxy.config.http.header_field_max_size setting limits individual header field sizes in the HTTP/1.1 parser, but the HPACK and QPACK decode paths had no equivalent check. This adds a max_string_len parameter to xpack_decode_string and passes the configured limit down through the HTTP/2 and HTTP/3 decode chains. * Address copilot comments * address copilot comments * Define 32768 as a constant
Instead of allocating arbitrary lengths on the stack, use the LocalBuffer class to manage stack vs heap allocated buffer. Note that the allocation is still limited by the records.yaml setting for proxy.config.http.request_line_max_size = 64KB So even without these fixes, the chance of blowing the stack is very limited, unless you have increased this setting.
HPACK indexes decoded into 64-bit values could be silently narrowed when looking up entries in the indexing table. A peer could encode a value above the 32-bit range that truncated to a different valid static table entry. This rejects oversized decoded indexes before any table lookup and keeps the narrowing explicit after validation. This also adds regression coverage for indexed and literal indexed-name representations. Co-authored-by: bneradt <bneradt@yahooinc.com>
Add config validation to cap round_robin_max_count to [1-1024] and a release assert in HostDBInfo::assign to catch srv_offset values that would silently truncate the 16-bit bitfield. The existing code comment documented the invariant but did not enforce it.
Update the records.yaml documentation to reflect the [1-1024] range constraint added in RecordsConfig. (cherry picked from commit 1cef3001bffc83de85d95af0769fd901e3112175)
Replace two VLAs sized by DNS response counts with ts::LocalBuffer. The SRV pointer array in dnsEvent and the live-targets array in select_best_srv used stack VLAs guarded only by ink_assert (compiled out in release). Use LocalBuffer with a stack size of 16 matching the default hostdb_round_robin_max_count, with automatic heap fallback for larger configurations.
Bounds-check type parameter in get_received_frame_count() for both Http2ClientSession and Http2ServerSession Clamp out-of-range values to HTTP2_FRAME_TYPE_MAX (the "unknown" bucket), matching the existing pattern in _count_received_frames on the write side
Add cp + RRFIXEDSZ > eom bounds check before NS_GET16/NS_GET32 reads of the fixed RR header fields (type, class, ttl, rdlength) Add dn_skipname error check and cp + SRV_FIXEDSZ > eom bounds check in the SRV record parsing path A truncated DNS UDP response could satisfy the cp < eom loop condition while multi-byte macro reads extend past the buffer end
Replace hand-rolled digit accumulation loops in mime_parse_int, mime_parse_uint, and mime_parse_int64 with std::from_chars to eliminate undefined behavior from signed integer overflow. Apply RFC-specific overflow policy per header type: Content-Length: reject as parse error (RFC 9112 §6.3) Age: clamp to 2^31 (RFC 9111 §1.2.2) Max-Forwards: clamp to INT32_MAX (RFC 9110 §7.6.2)
Co-authored-by: Serris Santos <lserris@apache.org>
Co-authored-by: Serris Santos <lserris@apache.org>
Client-controlled URL fields can make the WIPE_FIELD_VALUE log filter build a large temporary buffer while masking query parameter values. Keeping that buffer on the thread stack risks exhausting the stack and crashing traffic_server when the filter is configured for matching URL fields. This makes use of ts::LocalBuffer which uses stack space in the common case (header is under 8kb), but allocates memory if it is more than that. This keeps large marshalled URLs from consuming excessive thread stack space during WIPE_FIELD_VALUE processing.
PROXY protocol header parsing used a stack buffer sized from proxy.config.proxy_protocol.max_header_size. Operators can raise that limit for version 2 TLVs, which made the accept path reserve up to 64 KiB on event thread stacks and relied on a compiler VLA extension. This moves the scratch storage to a heap-backed buffer while keeping the configured maximum in effect. This also limits the copy to the bytes already available so normal traffic does not allocate more than the parser can inspect.
PROXY protocol v1 port fields arrive as text, and values outside the TCP port range could be narrowed into in_port_t before validation. Invalid input could therefore be treated as a different valid port in routing, ACL, and logging decisions. This validates the full decimal token in the parser before narrowing it to the port type. This also adds boundary coverage for accepted maximum ports and rejected zero, overflow, and truncating values.
Large tuple features and Host field rewrites can flow through txn_box from request-controlled input. Several paths used alloca with sizes derived from those values, which could consume a large fraction of the ATS thread stack or overflow it outright. This replaces those temporary alloca buffers with LocalBuffer so the common small cases stay local while larger request-controlled buffers move off the stack. The Host rewrite buffers keep the same ATS host-size local storage, with LocalBuffer providing a heap fallback when the formatted field is larger. This also corrects the Host port buffer sizing to use the integer digit capacity for in_port_t. max_digits10 is a floating-point round-trip trait and is zero for this integer type, so using digits10 + 1 reserves space for the largest decimal port value.
IPv6 address text with an oversized or signed hextet could be accepted by libswoc and narrowed into the 16-bit IPv6 quad storage. That allowed input such as 10000::1 to be interpreted as a different address after truncation. This parses IPv6 hextets as unsigned values and rejects any token that does not fit in one quad. This also adds coverage for the maximum valid hextet, oversized hextets, bracketed input, and signed tokens.
Large HPACK/QPACK integer encodings could wrap the accumulated value after each shifted continuation byte was individually accepted. That could turn an oversized encoded value into a small decoded integer and let later length checks see the wrong value. This adds cumulative overflow checks while preserving the existing invalid-length handling for overlong encodings. This also adds XPACK unit coverage for both the wrapping value and an encoding that would shift past the 64-bit range.
Dynamic HTTP/2 stream windows can emit a SETTINGS frame as streams open, and each unacknowledged frame keeps a local settings snapshot. A peer that keeps opening streams while withholding SETTINGS ACKs can grow that per-connection queue without bound. This caps outstanding SETTINGS snapshots based on the configured concurrent stream limit, skips non-preface no-op SETTINGS frames, and closes the connection with SETTINGS_TIMEOUT when the cap is exceeded. This also adds an h2 AuTest that withholds ACKs while continuing to read the connection.
Late shutdown can drop the global remap table before all net-thread work
has stopped accepting or initializing transactions. A transaction created in
that window can reach remap with an empty table lease and crash while
dereferencing it.
Addresses the following crash:
```
(gdb) bt
Backtrace stopped: Cannot access memory at address 0x7f9ba11f8418
(gdb) l
41 RemapProcessor::setup_for_remap(HttpTransact::State *s, UrlRewrite *table)
42 {
43 Dbg(dbg_ctl_url_rewrite, "setting up for remap: %p", s);
44 URL *request_url = nullptr;
45 bool mapping_found = false;
46 HTTPHdr *request_header = &s->hdr_info.client_request;
47 char **redirect_url = &s->remap_redirect;
48 const char *request_host;
49 int request_host_len;
50 int request_port;
(gdb)
```
This treats a missing table as a failed remap lookup and keeps the
transaction remap container in the same initialized no-mapping state used by
ordinary misses. This also covers the finish path, records the condition in
proxy.process.http.remap_missing_table, and emits warning-level diagnostics so
shutdown-window misses can be separated from ordinary no-match traffic.
Reverts apache#329 After some discussion wtih bcall, I think we can do a better fix than this.
Late shutdown can drop the global remap table before all net-thread work
has stopped accepting or initializing transactions. A transaction created in
that window can reach remap with an empty table lease and crash while
dereferencing it.
Addresses the following crash:
```
(gdb) bt
Backtrace stopped: Cannot access memory at address 0x7f9ba11f8418
(gdb) l
41 RemapProcessor::setup_for_remap(HttpTransact::State *s, UrlRewrite *table)
42 {
43 Dbg(dbg_ctl_url_rewrite, "setting up for remap: %p", s);
44 URL *request_url = nullptr;
45 bool mapping_found = false;
46 HTTPHdr *request_header = &s->hdr_info.client_request;
47 char **redirect_url = &s->remap_redirect;
48 const char *request_host;
49 int request_host_len;
50 int request_port;
(gdb)
```
This keeps the shutdown-window null table as a quiet defensive remap miss.
The guard runs before setup or finish dereferences the table, leaves
in-flight transactions that already hold a remap lease untouched, and avoids
warning or metric churn for a condition expected only while the process exits.
This intentionally leaves shutdown admission ordering unchanged. A broader
admission-gate fix can be evaluated separately from this cheap consumer
backstop.
ChunkedHandler::read_trailer() ended the trailer section on a bare LF blank line regardless of proxy.config.http.strict_chunk_parsing, while read_size() already required a full CRLF. Per RFC 9112 Section 7.1 the terminating empty line must be CRLF, so a bare LF let two parsers disagree on where the message ends. Reject it under strict parsing, mirroring read_size(); non-strict is unchanged. Adds unit and gold coverage.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When Traffic Server answers a request from its own cache without contacting an origin, any accompanying request body was left unread and, on a keep-alive connection, mis-parsed as the start of the next request. Drain the body where these internal responses converge; when it cannot be fully consumed (chunked, partial, or an active body tunnel) close the connection instead. Adds gold tests for the hit, miss, and drain-once paths. Co-authored-by: Leif Hedstrom <zwoop@apache.org>
The test declared Content-Type: image/png but sent a Netpbm/PPM body, relying on ImageMagick's own format sniffing to still hit the decode-side ResourceLimit. Combined with the signature check added on master, the mismatched body is now rejected before ever reaching ImageMagick, so the test no longer exercises the ResourceLimit path it was written to cover. Replace the fixture with a minimal WebP (VP8L) body whose declared type and actual signature agree, keeping the over-limit dimension so the ResourceLimit check still fires.
Two static_asserts referenced PCRE2_ENDANCHORED (added in PCRE2 10.30), which fails to compile on distros shipping older PCRE2 -- e.g., CentOS 7 ships PCRE2 10.23. Guard the assertions with #ifdef PCRE2_ENDANCHORED. The runtime path also breaks on old PCRE2: the RE_ENDANCHORED bit (0x20000000) is not a valid pcre2_compile option there and would be rejected with PCRE2_ERROR_BADOPTION, which would silently disable the CVE-2026-22068 fix in RemapConfig.cc that relies on compile(pattern, RE_ANCHORED | RE_ENDANCHORED) to require whole-hostname matches. Preserve the security semantics on old PCRE2 by transparently rewriting the pattern to "(?:pattern)\z" and stripping the bit before forwarding to pcre2_compile. This delegates end-of-subject anchoring to pcre2's own machinery, which correctly backtracks through alternation: for a pattern like "shortalt|longeralt" on a subject that only "longeralt" spans, pcre2 tries the first alternative, sees \z fail, and backtracks to try the second. A post-match length check would incorrectly stop at the first successful alternative. Modern PCRE2 (the common case) is untouched: RE_ENDANCHORED is passed through as PCRE2_ENDANCHORED natively, no allocation, no wrapping. The rewrite runs only when ATS_PCRE2_HAS_ENDANCHORED is false at compile time; the else branch is dead-eliminated by if constexpr on modern platforms. Error offsets returned from pcre2_compile are adjusted for the wrapper prefix so callers see offsets into their own pattern. Verified locally by forcing the fallback path and running probe tests against the alternation case (shorter alternative listed first followed by a longer one that spans the full subject) and single-pattern host allowlist patterns. Both branches match/reject correctly.
PR apache#350 (IncludeUrlValidator, CVE-2026-33988) added ts::tsutil to esicore's PUBLIC linkage because the new validator uses ts::Regex. This worked for esicore's own compilation, but because esicore is a STATIC library, that dependency propagates transitively to every consumer regardless of PRIVATE/PUBLIC on esicore -- static libraries don't hide link-time transitive deps. Consumers of esicore include the esi.so and combo_handler.so plugins, which are dlopen()ed alongside traffic_server. Traffic_server has libtsutil.a linked in, so it already has one copy of the tsutil globals (DbgCtl::_config_mode, the DbgCtl registry, etc.). With libtsutil.a also on each plugin's link line, each plugin embeds its own second copy of those globals. When ASan is enabled it detects the ODR violation at plugin load time and aborts: AddressSanitizer: odr-violation '_config_mode' at ../src/tsutil/DbgCtl.cc:254:18 [1] in combo_handler.so [2] in traffic_server Fix: remove ts::tsutil from esicore's linkage entirely. Unresolved tsutil symbols in esicore.a (from IncludeUrlValidator.cc's Regex calls) and in combo_handler.cc (DbgCtl usage) are left as undefined in the plugin and resolved by the runtime linker against traffic_server at dlopen time. This matches how every other plugin that uses DbgCtl (xdebug.so, header_rewrite.so, etc.) already handles tsutil references. esicore's tsutil headers remain visible via the top-level include_directories() in the project's root CMakeLists, so compilation still succeeds; only the link-time dependency is dropped. Test executables that link esicore (test_include_url_validator, test_processor, test_parser, test_vars, test_docnode) now need to link ts::tsutil explicitly, since executables cannot defer symbol resolution the way plugins can. Updated those targets to add ts::tsutil.
FreeBSD does not pull <sys/socket.h> transitively through <arpa/inet.h> or <netinet/in.h> the way glibc does, so AF_INET and AF_INET6 references in IncludeUrlValidator.cc's inet_pton() calls fail to resolve on that platform. Include <sys/socket.h> explicitly.
…yzer
setHostAllowRegex takes a const std::string &, and constructing a
std::string from a null char* is undefined behavior. getopt_long is
documented to supply optarg for required_argument options (which
--include-host-allow is), so this branch should never be null in
practice, but the clang analyzer cannot prove that and flags it as a
possible null-parameter passing:
cplusplus.StringChecker: The parameter must not be null
plugins/esi/esi.cc:1723
if (!pOptionInfo->url_validator.setHostAllowRegex(optarg)) {
Add an explicit null guard that fails closed via the same TSEmergency
path already used for compile failures. This preserves the security
contract (never silently disable the host allowlist) and satisfies the
analyzer.
Member
Author
|
[approve ci autest 0] |
curl 8.x can return CONNECT_FAILED (7) instead of RECV_ERROR (56) depending on when ATS closes the socket relative to curl reading the proxy response. The http_code=000/http_connect assertions still verify the ACL denial either way.
Member
Author
|
[approve ci Debian] |
Member
Author
|
[approve ci debian] |
ezelkow1
approved these changes
Jul 28, 2026
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.