diff --git a/CHANGELOG.md b/CHANGELOG.md index 19a2e326738..43d352174aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## main / unreleased +* [BUGFIX] Allow only valid trace ID characters when decoding [#854](https://github.com/grafana/tempo/pull/854) (@zalegrala) * [CHANGE] Upgrade Cortex from v1.9.0 to v1.9.0-131-ga4bf10354 [#841](https://github.com/grafana/tempo/pull/841) (@aknuds1) * [CHANGE] Change default tempo port from 3100 to 3200 [#770](https://github.com/grafana/tempo/pull/809) (@MurzNN) * [ENHANCEMENT] Added hedged request metric `tempodb_backend_hedged_roundtrips_total` and a new storage agnostic `tempodb_backend_request_duration_seconds` metric that diff --git a/pkg/util/http.go b/pkg/util/http.go index 5d8d041430c..a342b1bef93 100644 --- a/pkg/util/http.go +++ b/pkg/util/http.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/gorilla/mux" ) @@ -32,6 +33,18 @@ func ParseTraceID(r *http.Request) ([]byte, error) { } func hexStringToTraceID(id string) ([]byte, error) { + // The encoding/hex package does not handle non-hex characters. + // Ensure the ID has only the proper characters + for pos, idChar := range strings.Split(id, "") { + if (idChar >= "a" && idChar <= "f") || + (idChar >= "A" && idChar <= "F") || + (idChar >= "0" && idChar <= "9") { + continue + } else { + return nil, fmt.Errorf("trace IDs can only contain hex characters: invalid character '%s' at position %d", idChar, pos+1) + } + } + // the encoding/hex package does not like odd length strings. // just append a bit here if len(id)%2 == 1 { @@ -45,7 +58,7 @@ func hexStringToTraceID(id string) ([]byte, error) { size := len(byteID) if size > 16 { - return nil, errors.New("trace ids can't be larger than 128 bits") + return nil, errors.New("trace IDs can't be larger than 128 bits") } if size < 16 { byteID = append(make([]byte, 16-size), byteID...) diff --git a/pkg/util/http_test.go b/pkg/util/http_test.go index 7d5b871e859..ba21e294a95 100644 --- a/pkg/util/http_test.go +++ b/pkg/util/http_test.go @@ -1,6 +1,7 @@ package util import ( + "errors" "testing" "github.com/grafana/tempo/cmd/tempo-query/tempo" @@ -13,7 +14,7 @@ func TestHexStringToTraceID(t *testing.T) { tc := []struct { id string expected []byte - expectError bool + expectError error }{ { id: "12", @@ -30,20 +31,25 @@ func TestHexStringToTraceID(t *testing.T) { { id: "121234567890abcdef1234567890abcdef", // value too long expected: []byte{0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef, 0x12, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef}, - expectError: true, + expectError: errors.New("trace IDs can't be larger than 128 bits"), }, { id: "234567890abcdef", // odd length expected: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x34, 0x56, 0x78, 0x90, 0xab, 0xcd, 0xef}, }, + { + id: "1234567890abcdef ", // trailing space + expected: nil, + expectError: errors.New("trace IDs can only contain hex characters: invalid character ' ' at position 17"), + }, } for _, tt := range tc { t.Run(tt.id, func(t *testing.T) { actual, err := hexStringToTraceID(tt.id) - if tt.expectError { - assert.Error(t, err) + if tt.expectError != nil { + assert.Equal(t, tt.expectError, err) assert.Nil(t, actual) return }