Skip to content

Commit

Permalink
fix(baggage): validate chars panic with 0x80 (open-telemetry#5494)
Browse files Browse the repository at this point in the history
The validation rule for baggage key/values chars has a N+1 problem with
the unicode value: `0x80`.

For instance, `baggage.NewMemberRaw` could be called with a string value
including the rune `128` and return no error.

Then `baggage.New` would panic on `validateValueChar`:
```
=== RUN   TestValidateValueChar
--- FAIL: TestValidateValueChar (0.00s)
panic: runtime error: index out of range [128] with length 128 [recovered]
	panic: runtime error: index out of range [128] with length 128
```

---------

Co-authored-by: Sam Xie <sam@samxie.me>
  • Loading branch information
2 people authored and OrHayat committed Jun 23, 2024
1 parent 6ddfce1 commit 2e0014d
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Prevent random number generation data-race for experimental rand exemplars in `go.opentelemetry.io/otel/sdk/metric`. (#5456)
- Fix counting number of dropped attributes of `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5464)
- Run the `Detect` method in `go.opentelemetry.io/otel/sdk/resource` in parallel. (#5402)
- Fix panic in baggage creation when a member contains 0x80 char in key or value. (#5494)

## [1.27.0/0.49.0/0.3.0] 2024-05-21

Expand Down
4 changes: 2 additions & 2 deletions baggage/baggage.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ func validateKey(s string) bool {
}

func validateKeyChar(c int32) bool {
return c >= 0 && c <= int32(utf8.RuneSelf) && safeKeyCharset[c]
return c >= 0 && c < int32(utf8.RuneSelf) && safeKeyCharset[c]
}

func validateValue(s string) bool {
Expand Down Expand Up @@ -850,7 +850,7 @@ var safeValueCharset = [utf8.RuneSelf]bool{
}

func validateValueChar(c int32) bool {
return c >= 0 && c <= int32(utf8.RuneSelf) && safeValueCharset[c]
return c >= 0 && c < int32(utf8.RuneSelf) && safeValueCharset[c]
}

// valueEscape escapes the string so it can be safely placed inside a baggage value,
Expand Down
4 changes: 2 additions & 2 deletions baggage/baggage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestValidateKeyChar(t *testing.T) {
'\x10', '\x11', '\x12', '\x13', '\x14', '\x15', '\x16', '\x17',
'\x18', '\x19', '\x1A', '\x1B', '\x1C', '\x1D', '\x1E', '\x1F', ' ',
'(', ')', '<', '>', '@', ',', ';', ':', '\\', '"', '/', '[', ']', '?',
'=', '{', '}', '\x7F', 2 >> 20,
'=', '{', '}', '\x7F', 2 >> 20, '\x80',
}

for _, ch := range invalidKeyRune {
Expand All @@ -46,7 +46,7 @@ func TestValidateValueChar(t *testing.T) {
'\x08', '\x09', '\x0A', '\x0B', '\x0C', '\x0D', '\x0E', '\x0F',
'\x10', '\x11', '\x12', '\x13', '\x14', '\x15', '\x16', '\x17',
'\x18', '\x19', '\x1A', '\x1B', '\x1C', '\x1D', '\x1E', '\x1F', ' ',
'"', ',', ';', '\\', '\x7F',
'"', ',', ';', '\\', '\x7F', '\x80',
}

for _, ch := range invalidValueRune {
Expand Down

0 comments on commit 2e0014d

Please sign in to comment.