Skip to content

Commit

Permalink
Fix data race in HTTP request header sanitisation (#1159)
Browse files Browse the repository at this point in the history
* Fix data race in HTTP request header sanitisation

Don't update the header values slice, replace it.

* Update changelog
  • Loading branch information
axw authored Nov 25, 2021
1 parent 1ae5839 commit fafbf1a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ https://github.com/elastic/apm-agent-go/compare/v1.14.0...master[View commits]
- Discard exit spans shorter or equal than `ELASTIC_APM_EXIT_SPAN_MIN_DURATION`. Defaults to `1ms`. {pull}1138[#(1138)]
- module/apmprometheus: add support for mapping prometheus histograms. {pull}1145[#(1145)]
- Fixed a bug where errors in cloud metadata discovery could lead to the process aborting during initialisation {pull}1158[#(1158)]
- Fixed a data race related to HTTP request header sanitisation {pull}1159[#(1159)]
[[release-notes-1.x]]
=== Go Agent version 1.x
Expand Down
15 changes: 9 additions & 6 deletions sanitizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (

const redacted = "[REDACTED]"

var redactedValues = []string{redacted}

// sanitizeRequest sanitizes HTTP request data, redacting the
// values of cookies, headers and forms whose corresponding keys
// match any of the given wildcard patterns.
Expand All @@ -36,13 +38,11 @@ func sanitizeRequest(r *model.Request, matchers wildcard.Matchers) {
}
sanitizeHeaders(r.Headers, matchers)
if r.Body != nil && r.Body.Form != nil {
for key, values := range r.Body.Form {
for key := range r.Body.Form {
if !matchers.MatchAny(key) {
continue
}
for i := range values {
values[i] = redacted
}
r.Body.Form[key] = redactedValues
}
}
}
Expand All @@ -60,7 +60,10 @@ func sanitizeHeaders(headers model.Headers, matchers wildcard.Matchers) {
if !matchers.MatchAny(h.Key) || len(h.Values) == 0 {
continue
}
h.Values = h.Values[:1]
h.Values[0] = redacted
// h.Values may hold the original value slice from a
// net/http.Request, so it's important that we do not
// modify it. Instead, just replace the values with a
// shared, immutable slice.
h.Values = redactedValues
}
}

0 comments on commit fafbf1a

Please sign in to comment.