From e3037b28ff74e9f8383f94f6bb8b0b4f5c07bd66 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Sat, 29 Aug 2020 10:12:32 +0000 Subject: [PATCH 1/2] Check for unreasonable data in binary propagation Assume the data is corrupt and return an error instead of attempting to allocate hundreds of megabytes. Signed-off-by: Bryan Boreham --- propagation.go | 16 ++++++++++++++++ propagation_test.go | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/propagation.go b/propagation.go index f1acc560..98c2b789 100644 --- a/propagation.go +++ b/propagation.go @@ -215,6 +215,13 @@ func (p *BinaryPropagator) Inject( return nil } +// These are intended to be unimaginably high: above this number we declare +// the data is corrupted, to protect against running out of memory +const ( + maxBinaryBaggage = 1024 * 1024 + maxBinaryValueLen = 100 * 1024 * 1024 +) + // Extract implements Extractor of BinaryPropagator func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) { carrier, ok := abstractCarrier.(io.Reader) @@ -245,6 +252,9 @@ func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, er if err := binary.Read(carrier, binary.BigEndian, &numBaggage); err != nil { return emptyContext, opentracing.ErrSpanContextCorrupted } + if numBaggage > maxBinaryBaggage { + return emptyContext, opentracing.ErrSpanContextCorrupted + } if iNumBaggage := int(numBaggage); iNumBaggage > 0 { ctx.baggage = make(map[string]string, iNumBaggage) buf := p.buffers.Get().(*bytes.Buffer) @@ -255,6 +265,9 @@ func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, er if err := binary.Read(carrier, binary.BigEndian, &keyLen); err != nil { return emptyContext, opentracing.ErrSpanContextCorrupted } + if keyLen > maxBinaryValueLen { + return emptyContext, opentracing.ErrSpanContextCorrupted + } buf.Reset() buf.Grow(int(keyLen)) if n, err := io.CopyN(buf, carrier, int64(keyLen)); err != nil || int32(n) != keyLen { @@ -265,6 +278,9 @@ func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, er if err := binary.Read(carrier, binary.BigEndian, &valLen); err != nil { return emptyContext, opentracing.ErrSpanContextCorrupted } + if valLen > maxBinaryValueLen { + return emptyContext, opentracing.ErrSpanContextCorrupted + } buf.Reset() buf.Grow(int(valLen)) if n, err := io.CopyN(buf, carrier, int64(valLen)); err != nil || int32(n) != valLen { diff --git a/propagation_test.go b/propagation_test.go index 5f25a821..2bfc3b54 100644 --- a/propagation_test.go +++ b/propagation_test.go @@ -16,7 +16,9 @@ package jaeger import ( "bytes" + "encoding/base64" "net/http" + "strings" "testing" "time" @@ -127,6 +129,26 @@ func TestSpanPropagator(t *testing.T) { }...) } +func TestBinaryCorruption(t *testing.T) { + const op = "test" + reporter := NewInMemoryReporter() + tracer, closer := NewTracer("x", NewConstSampler(true), reporter) + + sp := tracer.StartSpan(op) + carrier := new(bytes.Buffer) + err := tracer.Inject(sp.Context(), opentracing.Binary, carrier) + assert.NoError(t, err) + + // suppose we encode the data then forget to decode it + data := base64.StdEncoding.EncodeToString(carrier.Bytes()) + _, err = tracer.Extract(opentracing.Binary, strings.NewReader(data)) + if assert.Error(t, err) { + assert.Equal(t, opentracing.ErrSpanContextCorrupted, err) + } + sp.Finish() + closer.Close() +} + func TestSpanIntegrityAfterSerialize(t *testing.T) { serializedString := "f6c385a2c57ed8d7:00b04a90b7723bdc:76c385a2c57ed8d7:1" From 0e9572623de3d750ac3b21c66515bfe65923477d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 17 Mar 2021 14:55:22 +0000 Subject: [PATCH 2/2] Change to W3C limits Signed-off-by: Bryan Boreham --- propagation.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/propagation.go b/propagation.go index 98c2b789..e06459b9 100644 --- a/propagation.go +++ b/propagation.go @@ -215,11 +215,10 @@ func (p *BinaryPropagator) Inject( return nil } -// These are intended to be unimaginably high: above this number we declare -// the data is corrupted, to protect against running out of memory +// W3C limits https://github.com/w3c/baggage/blob/master/baggage/HTTP_HEADER_FORMAT.md#limits const ( - maxBinaryBaggage = 1024 * 1024 - maxBinaryValueLen = 100 * 1024 * 1024 + maxBinaryBaggage = 180 + maxBinaryNameValueLen = 4096 ) // Extract implements Extractor of BinaryPropagator @@ -265,9 +264,6 @@ func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, er if err := binary.Read(carrier, binary.BigEndian, &keyLen); err != nil { return emptyContext, opentracing.ErrSpanContextCorrupted } - if keyLen > maxBinaryValueLen { - return emptyContext, opentracing.ErrSpanContextCorrupted - } buf.Reset() buf.Grow(int(keyLen)) if n, err := io.CopyN(buf, carrier, int64(keyLen)); err != nil || int32(n) != keyLen { @@ -278,7 +274,7 @@ func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, er if err := binary.Read(carrier, binary.BigEndian, &valLen); err != nil { return emptyContext, opentracing.ErrSpanContextCorrupted } - if valLen > maxBinaryValueLen { + if keyLen+valLen > maxBinaryNameValueLen { return emptyContext, opentracing.ErrSpanContextCorrupted } buf.Reset()