fix: Enforce W3C Baggage limits on the extract path - #2298
Open
serhiy-bzhezytskyy wants to merge 3 commits into
Open
fix: Enforce W3C Baggage limits on the extract path#2298serhiy-bzhezytskyy wants to merge 3 commits into
serhiy-bzhezytskyy wants to merge 3 commits into
Conversation
The inject path (#encode) applies MAX_ENTRIES, MAX_ENTRY_LENGTH and MAX_TOTAL_LENGTH, but extract parsed the inbound header with none of them, decoding an unbounded baggage header in full — the class the Java runtime tracks as CVE-2026-45292. extract now mirrors the inject limits through a private helper: over-limit entries are dropped at the point the limit is reached and already-decoded entries are kept. Adds three extract-side tests mirroring the existing inject-side limit tests. Fixes open-telemetry#2163 Assisted-By: Claude Fable 5
serhiy-bzhezytskyy
requested review from
a team,
dazuma,
fbogsany,
kaylareopelle,
mwear,
robbkidd,
robertlaurin and
xuan-cao-swi
as code owners
August 17, 2026 16:14
|
|
simi
approved these changes
Aug 18, 2026
Contributor
|
notice this still reads full header into memory |
decode_entries already stopped after 180 entries, but the header was split eagerly first, so a large header materialised every entry before any limit applied. Walk it lazily and stop at the entry limit instead. On a 7.8 MB header with 500k entries that is 180 strings instead of 500,000. Whitespace is now stripped per entry rather than over the whole header, which keeps the parsing semantics and drops a copy of the header. Empty entries are skipped explicitly: without the global gsub a whitespace-only header yields one empty entry, which raised inside decode_entries and was swallowed by the rescue. Assisted-By: Claude Fable 5
The allocation-count test covered the bound only indirectly. Extract a 1,000-entry header and a 100,000-entry one and assert the second allocates less than twice the first, so the assertion is relative and does not depend on a Ruby version's absolute counts. Assisted-By: Claude Fable 5
Author
|
Thanks, you're right. Fixed and covered it with two tests: extract now walks the header lazily and stops at 180 entries, so nothing past the 180th is scanned or copied. The tests assert the property, that allocations do not grow with the header length; the numbers I measured separately are 180 strings instead of 500,000 and 0.04 ms instead of 53 ms on a 7.8 MB header with 500k entries. |
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
2 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.
The
baggageextract path decodes the inbound header without applying theMAX_ENTRIES(180),MAX_ENTRY_LENGTH(4096) andMAX_TOTAL_LENGTH(8192) limits that the inject path (#encode) enforces, so an unbounded inbound header is parsed in full. This is the class the Java runtime fixed as CVE-2026-45292 (unbounded memory/CPU via W3C Baggage parsing, fixed in opentelemetry-java 1.62.0). Fixes #2163.extractnow enforces the same three limits through a small private helper mirroring#encode: over-limit entries are dropped where the limit is reached, and the entries decoded before that are kept. Three extract-side tests mirror the existing inject-side limit tests; each fails onmainand passes with the change.Assisted-By: Claude Fable 5