From 282fcdd366d8041cf1bd7a3656e6b4198a6d4494 Mon Sep 17 00:00:00 2001 From: serhiy-bzhezytskyy Date: Mon, 17 Aug 2026 19:11:21 +0300 Subject: [PATCH 1/3] fix: Enforce W3C Baggage limits on the extract path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #2163 Assisted-By: Claude Fable 5 --- .../propagation/text_map_propagator.rb | 28 ++++++++++++----- .../propagation/text_map_propagator_test.rb | 31 +++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb b/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb index 3e3e699502..1be16c7ebb 100644 --- a/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb +++ b/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb @@ -58,14 +58,7 @@ def extract(carrier, context: Context.current, getter: Context::Propagation.text entries = header.gsub(/\s/, '').split(',') OpenTelemetry::Baggage.build(context: context) do |builder| - entries.each do |entry| - # Note metadata is currently unused in OpenTelemetry, but is part - # the W3C spec where it's referred to as properties. We preserve - # the properties (as-is) so that they can be propagated elsewhere. - kv, meta = entry.split(';', 2) - k, v = kv.split('=').map! { |part| URI.decode_uri_component(part) } - builder.set_value(k, v, metadata: meta) - end + decode_entries(entries, builder) end rescue StandardError => e OpenTelemetry.logger.debug "Error extracting W3C baggage: #{e.message}" @@ -82,6 +75,25 @@ def fields private + def decode_entries(entries, builder) + decoded_count = 0 + decoded_length = 0 + entries.each do |entry| + break unless decoded_count < MAX_ENTRIES + next unless entry.size <= MAX_ENTRY_LENGTH && + entry.size + decoded_length <= MAX_TOTAL_LENGTH + + # Note metadata is currently unused in OpenTelemetry, but is part + # the W3C spec where it's referred to as properties. We preserve + # the properties (as-is) so that they can be propagated elsewhere. + kv, meta = entry.split(';', 2) + k, v = kv.split('=').map! { |part| URI.decode_uri_component(part) } + builder.set_value(k, v, metadata: meta) + decoded_count += 1 + decoded_length += entry.size + 1 # +1 for the ',' separator, as in #encode + end + end + def encode(baggage) result = +'' encoded_count = 0 diff --git a/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb b/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb index 9d5187cea9..320de52a3b 100644 --- a/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb +++ b/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb @@ -70,6 +70,37 @@ _(context.object_id).wont_equal(empty_context.object_id) end end + + describe 'enforced limits' do + it 'enforces max of 180 name-value pairs' do + header = (0..180).map { |i| "k#{i}=v#{i}" }.join(',') + context = propagator.extract({ header_key => header }, context: Context.empty) + + 180.times { |i| _(OpenTelemetry::Baggage.value("k#{i}", context: context)).must_equal("v#{i}") } + _(OpenTelemetry::Baggage.value('k180', context: context)).must_be_nil + end + + it 'enforces max entry length of 4096' do + oversized = "key1=#{'x' * 4092}" # 4097 chars including 'key1=' + header = "#{oversized},key2=val2" + context = propagator.extract({ header_key => header }, context: Context.empty) + + _(OpenTelemetry::Baggage.value('key1', context: context)).must_be_nil + _(OpenTelemetry::Baggage.value('key2', context: context)).must_equal('val2') + end + + it 'enforces total length of 8192 chars' do + # each entry is 100 chars including '=' and ','; 82 entries would be 8199 > 8192 + keys = (0..81).map { |i| "k#{i.to_s.rjust(48, '0')}" } + values = (0..81).map { |i| "v#{i.to_s.rjust(48, '0')}" } + header = keys.zip(values).map { |k, v| "#{k}=#{v}" }.join(',') + + context = propagator.extract({ header_key => header }, context: Context.empty) + + 81.times { |i| _(OpenTelemetry::Baggage.value(keys[i], context: context)).wont_be_nil } + _(OpenTelemetry::Baggage.value(keys.last, context: context)).must_be_nil + end + end end describe '#inject' do From 63d9d3b2ef19053f942942887eeef2ffa1a82967 Mon Sep 17 00:00:00 2001 From: serhiy-bzhezytskyy Date: Wed, 19 Aug 2026 00:35:22 +0300 Subject: [PATCH 2/3] Bound the baggage extract work, not just its result 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 --- .../baggage/propagation/text_map_propagator.rb | 9 ++++----- .../baggage/propagation/text_map_propagator_test.rb | 12 ++++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb b/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb index 1be16c7ebb..9db8be90ab 100644 --- a/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb +++ b/api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb @@ -55,7 +55,7 @@ def extract(carrier, context: Context.current, getter: Context::Propagation.text header = getter.get(carrier, BAGGAGE_KEY) return context if header.nil? || header.empty? - entries = header.gsub(/\s/, '').split(',') + entries = header.each_line(',', chomp: true).lazy.take(MAX_ENTRIES) OpenTelemetry::Baggage.build(context: context) do |builder| decode_entries(entries, builder) @@ -76,10 +76,10 @@ def fields private def decode_entries(entries, builder) - decoded_count = 0 decoded_length = 0 - entries.each do |entry| - break unless decoded_count < MAX_ENTRIES + entries.each do |raw_entry| + entry = raw_entry.gsub(/\s/, '') + next if entry.empty? next unless entry.size <= MAX_ENTRY_LENGTH && entry.size + decoded_length <= MAX_TOTAL_LENGTH @@ -89,7 +89,6 @@ def decode_entries(entries, builder) kv, meta = entry.split(';', 2) k, v = kv.split('=').map! { |part| URI.decode_uri_component(part) } builder.set_value(k, v, metadata: meta) - decoded_count += 1 decoded_length += entry.size + 1 # +1 for the ',' separator, as in #encode end end diff --git a/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb b/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb index 320de52a3b..306976f8e0 100644 --- a/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb +++ b/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb @@ -72,6 +72,18 @@ end describe 'enforced limits' do + it 'does not materialise every entry of an oversized header' do + header = (0...50_000).map { |i| "k#{i}=v#{i}" }.join(',') + carrier = { 'baggage' => header } + + before = GC.stat[:total_allocated_objects] + context = propagator.extract(carrier, context: OpenTelemetry::Context.empty) + allocated = GC.stat[:total_allocated_objects] - before + + _(OpenTelemetry::Baggage.values(context: context).size).must_equal(180) + _(allocated).must_be(:<, 10_000) + end + it 'enforces max of 180 name-value pairs' do header = (0..180).map { |i| "k#{i}=v#{i}" }.join(',') context = propagator.extract({ header_key => header }, context: Context.empty) From bfd9fdf1c7737b1dc33491f986543823336beca3 Mon Sep 17 00:00:00 2001 From: serhiy-bzhezytskyy Date: Wed, 19 Aug 2026 01:06:13 +0300 Subject: [PATCH 3/3] Assert that extract work does not grow with the header length 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 --- .../propagation/text_map_propagator_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb b/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb index 306976f8e0..6fb7d0d767 100644 --- a/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb +++ b/api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb @@ -72,6 +72,22 @@ end describe 'enforced limits' do + it 'does work proportional to the limit, not to the header length' do + def allocations_for(entry_count) + header = (0...entry_count).map { |i| "k#{i}=v#{i}" }.join(',') + carrier = { 'baggage' => header } + before = GC.stat[:total_allocated_objects] + propagator.extract(carrier, context: OpenTelemetry::Context.empty) + GC.stat[:total_allocated_objects] - before + end + + small = allocations_for(1_000) + large = allocations_for(100_000) + + # A hundredfold longer header must not cost a hundredfold more work. + _(large).must_be(:<, small * 2) + end + it 'does not materialise every entry of an oversized header' do header = (0...50_000).map { |i| "k#{i}=v#{i}" }.join(',') carrier = { 'baggage' => header }