Backport Release 2 security bundle (#13442) to 10.2.x - #13452
Merged
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. (cherry picked from commit f910eed)
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. (cherry picked from commit 79d2d8d)
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. (cherry picked from commit 48e967d)
* 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 (cherry picked from commit 12f9ba5)
* Close connection if unneccesary H2 SETTINGS frame arrives * Address copilot comment (cherry picked from commit 6375409)
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. (cherry picked from commit eca51e9)
* 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 (cherry picked from commit 24e04b7)
* 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 (cherry picked from commit f4561a9)
* 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 (cherry picked from commit 4782ee1)
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. (cherry picked from commit 6b5d7ce)
(cherry picked from commit f994194)
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> (cherry picked from commit 4b971b5)
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. (cherry picked from commit 1096563)
Update the records.yaml documentation to reflect the [1-1024] range constraint added in RecordsConfig. (cherry picked from commit 1cef3001bffc83de85d95af0769fd901e3112175) (cherry picked from commit c45594f)
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. (cherry picked from commit 8875ab4)
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 (cherry picked from commit 2188276)
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 (cherry picked from commit d9bed82)
(cherry picked from commit 1283b37)
(cherry picked from commit 6068623)
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) (cherry picked from commit 422b978)
Co-authored-by: Serris Santos <lserris@apache.org> (cherry picked from commit 0e8272d)
Co-authored-by: Serris Santos <lserris@apache.org> (cherry picked from commit e8d92a5)
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. (cherry picked from commit f733942)
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. (cherry picked from commit 416e5b1)
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. (cherry picked from commit 94255c1)
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. (cherry picked from commit 1960c44)
(cherry picked from commit 554867f)
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. (cherry picked from commit e4bb476)
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. (cherry picked from commit 292679e)
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. (cherry picked from commit d4775d1)
CONNECT destinations to loopback or private services need an explicit destination policy in addition to port allowlisting to protect them. Operators already have that control through outbound ip_allow rules, but the default files and coverage did not fully show how to protect local resources, alternate unspecified or IPv4-mapped address forms, or how to opt in to intentional local tunnels. This is a default configuration, documentation, and test change only; it does not add new core CONNECT validation code. It adds CONNECT-only outbound denies for unspecified, loopback, private, link-local, and IPv4-mapped IPv6 destination ranges, while keeping intentional deployments overrideable with earlier outbound allow rules. (cherry picked from commit 67286ef)
When C= has no trailing delimiter, strstr() returns nullptr and the (pp - cp) arithmetic was undefined behavior. Add a nullptr check in both the AF_INET and AF_INET6 branches, plus autest cases. (cherry picked from commit 6451527)
ATS uses @ headers as a mechanism for internal metadata. They are not allowed by the RFC and should not appear externally, either from the client or server nor to the client or server. This therefore strips client request and origin response @ headers before READ_REQUEST_HDR and READ_RESPONSE_HDR hooks run so plugins cannot observe such internal headers from untrusted peers which may confuse the plugins. This also extends ATSReplayTest replay handling so tests can stage custom plugins and pass structured plugin arguments directly from replay configuration. Log and count when ATS strips untrusted @ headers from client requests and origin responses before plugin hooks. This gives operators visibility into attempts to inject internal @ metadata. (cherry picked from commit 4ab63db)
A pre-session inactivity/handshake timer left armed across the TLS handoff could fire while a deferred TS_HTTP_SSN_START_HOOK callout is pending, delivering VC_EVENT_INACTIVITY_TIMEOUT to a session acceptor and release-asserting traffic_server. Cancel the timer before handing off the read VIO and re-arm accept_no_activity_timeout only after the start hooks return. Adds a regression autest. (cherry picked from commit 1eb2e7b)
regex_map compiled its host pattern unanchored, so a rule for "cdn.example.com" also matched "prefix.cdn.example.com" and "cdn.example.com.test". Compile with RE_ANCHORED | RE_ENDANCHORED so the pattern requires a whole-host match; it now matches only the exact host. (cherry picked from commit a074ce5)
(cherry picked from commit f474ebb)
(cherry picked from commit 8441511)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit df60315)
(cherry picked from commit ff6992b)
(cherry picked from commit 810355d)
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.
(cherry picked from commit 0257fc3)
Reverts apache#329 After some discussion wtih bcall, I think we can do a better fix than this. (cherry picked from commit 258cf64)
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.
(cherry picked from commit 5119d93)
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. (cherry picked from commit 043fb22)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 7d0b435)
(cherry picked from commit 42d1991)
(cherry picked from commit 867f2b5)
(cherry picked from commit 7e22656)
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> (cherry picked from commit 337816b)
(cherry picked from commit 3dca68b)
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. (cherry picked from commit 5224ac1)
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. (cherry picked from commit b275158)
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. (cherry picked from commit 9118cc8)
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. (cherry picked from commit b395a08)
…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.
(cherry picked from commit c611f80)
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. (cherry picked from commit e7051e1)
The global remap table's load and acquire ran as two unsynchronized steps, while config reload swapped the table and immediately released the old one without a mutex. A reader preempted between load and acquire could revive a table whose refcount the reload had just driven to zero, after the deleter was already scheduled. Retire the bespoke acquire/release refcount on UrlRewrite and let std::atomic<std::shared_ptr<UrlRewrite>> (via the new AtomicSharedPtr helper) own the publish-and-replace. Each transaction snapshots the current table into HttpSM::m_remap on session start; reload exchange()s in a new shared_ptr and drops its ref, so the old table destructs only after the last in-flight HttpSM releases its snapshot. Add shutdown_url_rewrite() to drain and inhibit further drops so plugin doneInstance() runs while this_ethread() is still valid. Co-authored-by: Masaori Koshiba <masaori@apache.org>
Contributor
Author
|
[approve ci fedora] |
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
13 participants
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.
Draft — CI verification only
Backports the #13442 "[master] Release 2" coordinated security/hardening bundle to
10.2.x(these fixes are what the 10.2.0 release is waiting on). Cherry-picked with-xfrom the range
f910eed635^..e7051e1583— 149 of 150 commits.Security changes are documented in the separate security document (not the
milestone-driven changelog), so there is no per-PR milestone/project bookkeeping here.
Included: remap-table refcount race fix
709443e870"Fix race in remap table refcount during reload" (thestd::atomic<UrlRewrite*>→AtomicSharedPtr<UrlRewrite>migration) is now folded intothis PR, adapted to 10.2.x's
register_config/CfgLoadreload code and scoped enums. Theshutdown-teardown guard it pairs with is already in this bundle.
Commits that required manual conflict resolution
Master SHA → on-branch SHA. The first two are code-logic and worth review; the rest are
new-unit-test registration / test-file merges (10.2.x lacked master-only tests
test_PluginYAML.cc/benchmark_TLSCertCompression.cc, which were excluded).2ce51ba62c→94f2069783— SSLNetVConnection: applied the proxy-protocolsrc/dst-addr null-guard onto 10.2.x's existing control flow (did not take master's
refactored parse-failure branch).
e8d92a51f2→0a9b3a3dd5— test_Hdrs.cc: appended only the newhttp_parse_statusTEST_CASE; excluded the master-only
unmarshal_v24_1block.8102a6b6e3→e6f8b84551— proxy unit_tests CMakeLists: addedtest_ControlBase.cc.6a4ecdad19→21b786a1ec— net unit_tests CMakeLists: addedtest_OCSPStapling.cc.6a8c575f3f→de964687af— proxy unit_tests CMakeLists: addedtest_FetchSM.cc.6916a91bde→63d5c95765— test_proxy: the macOS link-duplicates fix (droplibinknet_stub.cc, add
appVersionInfo).b5510fb26d→93657ab769and43a5d7dec7→c17b724772—http2_crlf_header_validation.test.py (records_config format interplay with the ssl fixup).
AuTest fixups (10.2.x harness adaptation)
16 tests used the
ssl_multicert.yamlDisk API, which 10.2.x's autest harness/ATS don'tprovide; converted each to the flat
ssl_multicert.configform (same as #13448) and foldedthe conversion into its originating commit. Files: h2/{h2_origin_cert_reverify,
h2_origin_trailers_h1, http2_crlf_header_validation, http2_flow_control,
http2_txn_start_read_gate, http2}, headers/oversized_field_h2,
ip_allow/connect_destination_acl, pluginTest/{certifier, multiplexer,
rate_limit/rate_limit_iprep, rate_limit/rate_limit_sni,
webp_transform/webp_transform_size_cap}, tls/{proxy_protocol_addressless,
tls_accept_timeout_crash, tls_engine_abort}.
Local: full build clean (macOS); ctest 162/163 (only the pre-existing macOS
test_jsonrpcserverunix-socket flake fails).