From fafbf1a9b762474b517911a307a35a8bbb5e4ca9 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 25 Nov 2021 11:00:18 +0800 Subject: [PATCH] Fix data race in HTTP request header sanitisation (#1159) * Fix data race in HTTP request header sanitisation Don't update the header values slice, replace it. * Update changelog --- CHANGELOG.asciidoc | 1 + sanitizer.go | 15 +++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 7ce83f3ef..6314a72d9 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -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 diff --git a/sanitizer.go b/sanitizer.go index 6168c0b76..e80d485d4 100644 --- a/sanitizer.go +++ b/sanitizer.go @@ -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. @@ -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 } } } @@ -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 } }