Merge commit from fork

* fix(propagation): limit baggage extraction error reporting Use sync.Once when reporting malformed or oversized baggage headers so attacker-controlled extraction failures cannot repeatedly flood the global error handler/log output. * propagation: test baggage error reporting limit * add changelog entry
2026-06-19 21:45:50 +02:00 · 2026-05-26 19:43:43 +02:00
parent 36c2f1bfd1
commit f02feacf86
4 changed files with 112 additions and 16 deletions
@@ -91,6 +91,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Close schema files opened by `ParseFile` in `go.opentelemetry.io/otel/schema/v1.0` and `go.opentelemetry.io/otel/schema/v1.1`. ([GHSA-995v-fvrw-c78m](https://github.com/open-telemetry/opentelemetry-go/security/advisories/GHSA-995v-fvrw-c78m))
 - Enforce the 8192-byte baggage size limit during extraction/parsing, changing behavior when the limit is exceeded in `go.opentelemetry.io/otel/baggage` and `go.opentelemetry.io/otel/propagation`. (#8222)
 - Fix `go.opentelemetry.io/otel/semconv/v1.41.0` to include `Attr*` helper methods for required attributes on observable instruments. (#8361)
+- Limit baggage extraction error reporting in `go.opentelemetry.io/otel/propagation` to prevent malformed or oversized baggage headers from flooding logs. ([GHSA-5wrp-cwcj-q835](https://github.com/open-telemetry/opentelemetry-go/security/advisories/GHSA-5wrp-cwcj-q835))

 <!-- Released section -->
 <!-- Don't change this section unless doing release -->
@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sync"

 	"go.opentelemetry.io/otel/baggage"
 	"go.opentelemetry.io/otel/internal/errorhandler"
@@ -23,6 +24,10 @@ const (
 	maxBytesPerBaggageString = 8192
 )

+// handleExtractErrOnce limits error reporting for attacker-controlled baggage headers
+// to one process-wide emission, preventing repeated extraction from flooding logs.
+var handleExtractErrOnce sync.Once
+
 // Baggage is a propagator that supports the W3C Baggage format.
 //
 // This propagates user-defined baggage associated with a trace. The complete
@@ -62,7 +67,9 @@ func extractSingleBaggage(parent context.Context, carrier TextMapCarrier) contex

 	bag, err := baggage.Parse(bStr)
 	if err != nil {
-		errorhandler.GetErrorHandler().Handle(err)
+		handleExtractErrOnce.Do(func() {
+			errorhandler.GetErrorHandler().Handle(err)
+		})
 	}
 	if bag.Len() == 0 {
 		return parent
@@ -91,11 +98,13 @@ func extractMultiBaggage(parent context.Context, carrier ValuesGetter) context.C
 			// individually. Mirror the single-header behavior of
 			// reporting the error and returning the parent context
 			// with no baggage attached.
-			errorhandler.GetErrorHandler().Handle(fmt.Errorf(
-				"baggage: aggregate header size %d exceeds %d byte limit",
-				totalBytes,
-				maxBytesPerBaggageString,
-			))
+			handleExtractErrOnce.Do(func() {
+				errorhandler.GetErrorHandler().Handle(fmt.Errorf(
+					"baggage: aggregate header size %d exceeds %d byte limit",
+					totalBytes,
+					maxBytesPerBaggageString,
+				))
+			})
 			return parent
 		}

@@ -124,7 +133,9 @@ func extractMultiBaggage(parent context.Context, carrier ValuesGetter) context.C
 		truncateErr = errors.Join(truncateErr, err)
 	}
 	if truncateErr != nil {
-		errorhandler.GetErrorHandler().Handle(truncateErr)
+		handleExtractErrOnce.Do(func() {
+			errorhandler.GetErrorHandler().Handle(truncateErr)
+		})
 	}

 	if b.Len() == 0 {
@@ -539,10 +539,29 @@ func TestExtractOversizedSingleBaggageHeader(t *testing.T) {
 }

 type errHandler struct {
-	err error
+	err   error
+	count int
 }

-func (e *errHandler) Handle(err error) { e.err = err }
+func (e *errHandler) Handle(err error) {
+	e.err = err
+	e.count++
+}
+
+func setBaggageErrHandler(t *testing.T) *errHandler {
+	t.Helper()
+
+	propagation.ResetHandleExtractErrOnce()
+	originalErrorHandler := otel.GetErrorHandler()
+	eh := &errHandler{}
+	otel.SetErrorHandler(eh)
+	t.Cleanup(func() {
+		otel.SetErrorHandler(originalErrorHandler)
+		propagation.ResetHandleExtractErrOnce()
+	})
+
+	return eh
+}

 func TestExtractManyBaggageHeader(t *testing.T) {
 	tests := []struct {
@@ -619,10 +638,7 @@ func TestExtractManyBaggageHeader(t *testing.T) {

 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			originalErrorHandler := otel.GetErrorHandler()
-			eh := &errHandler{}
-			otel.SetErrorHandler(eh)
-			t.Cleanup(func() { otel.SetErrorHandler(originalErrorHandler) })
+			eh := setBaggageErrHandler(t)

 			prop := propagation.Baggage{}
 			req, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://example.com", http.NoBody)
@@ -634,9 +650,66 @@ func TestExtractManyBaggageHeader(t *testing.T) {
 			got := baggage.FromContext(ctx)

 			assert.Equal(t, tt.want().Baggage(t), got)
-			assert.Error(t, eh.err)
-			for _, s := range tt.wantErrStr {
-				assert.Contains(t, eh.err.Error(), s)
+			if assert.Error(t, eh.err) {
+				for _, s := range tt.wantErrStr {
+					assert.Contains(t, eh.err.Error(), s)
+				}
+			}
+		})
+	}
+}
+
+func TestExtractInvalidBaggageReportsErrorOnce(t *testing.T) {
+	tests := []struct {
+		name    string
+		carrier func(t *testing.T) propagation.TextMapCarrier
+		wantErr string
+	}{
+		{
+			name: "single header parse error",
+			carrier: func(t *testing.T) propagation.TextMapCarrier {
+				t.Helper()
+				return propagation.MapCarrier{"baggage": "invalid"}
+			},
+			wantErr: "invalid baggage list-member",
+		},
+		{
+			name: "multiple header parse errors",
+			carrier: func(t *testing.T) propagation.TextMapCarrier {
+				t.Helper()
+				req, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://example.com", http.NoBody)
+				req.Header.Add("baggage", "invalid")
+				return propagation.HeaderCarrier(req.Header)
+			},
+			wantErr: "invalid baggage list-member",
+		},
+		{
+			name: "aggregate header size error",
+			carrier: func(t *testing.T) propagation.TextMapCarrier {
+				t.Helper()
+				req, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://example.com", http.NoBody)
+				req.Header.Add("baggage", "k="+strings.Repeat("v", maxBytesPerBaggageString/2-2))
+				req.Header.Add("baggage", "y="+strings.Repeat("v", maxBytesPerBaggageString/2-2))
+				return propagation.HeaderCarrier(req.Header)
+			},
+			wantErr: "exceeds 8192 byte limit",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			eh := setBaggageErrHandler(t)
+			prop := propagation.Baggage{}
+
+			for range 10 {
+				ctx := prop.Extract(t.Context(), tt.carrier(t))
+				got := baggage.FromContext(ctx)
+				assert.Equal(t, 0, got.Len(), "invalid header should result in empty baggage")
+			}
+
+			assert.Equal(t, 1, eh.count, "invalid baggage extraction should report only once")
+			if assert.Error(t, eh.err) {
+				assert.Contains(t, eh.err.Error(), tt.wantErr)
 			}
 		})
 	}
@@ -0,0 +1,11 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package propagation
+
+import "sync"
+
+// ResetHandleExtractErrOnce resets handleExtractErrOnce for tests.
+func ResetHandleExtractErrOnce() {
+	handleExtractErrOnce = sync.Once{}
+}