Skip to content

Commit

Permalink
Check for unreasonable data in binary propagation (#529)
Browse files Browse the repository at this point in the history
* 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 <bjboreham@gmail.com>

* Change to W3C limits

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
  • Loading branch information
bboreham committed Apr 11, 2021
1 parent 967f9c3 commit 99ac6dc
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
12 changes: 12 additions & 0 deletions propagation.go
Expand Up @@ -215,6 +215,12 @@ func (p *BinaryPropagator) Inject(
return nil
}

// W3C limits https://github.com/w3c/baggage/blob/master/baggage/HTTP_HEADER_FORMAT.md#limits
const (
maxBinaryBaggage = 180
maxBinaryNameValueLen = 4096
)

// Extract implements Extractor of BinaryPropagator
func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, error) {
carrier, ok := abstractCarrier.(io.Reader)
Expand Down Expand Up @@ -245,6 +251,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)
Expand All @@ -265,6 +274,9 @@ func (p *BinaryPropagator) Extract(abstractCarrier interface{}) (SpanContext, er
if err := binary.Read(carrier, binary.BigEndian, &valLen); err != nil {
return emptyContext, opentracing.ErrSpanContextCorrupted
}
if keyLen+valLen > maxBinaryNameValueLen {
return emptyContext, opentracing.ErrSpanContextCorrupted
}
buf.Reset()
buf.Grow(int(valLen))
if n, err := io.CopyN(buf, carrier, int64(valLen)); err != nil || int32(n) != valLen {
Expand Down
22 changes: 22 additions & 0 deletions propagation_test.go
Expand Up @@ -16,7 +16,9 @@ package jaeger

import (
"bytes"
"encoding/base64"
"net/http"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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"

Expand Down

0 comments on commit 99ac6dc

Please sign in to comment.