From 4e814a4209e6b34cad313063455b88e4c90e31ea Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Wed, 17 Oct 2018 13:27:10 -0700 Subject: [PATCH 1/2] Disallow multiple values for Content-Type header --- rollbar_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++++++++ transforms.go | 33 ++++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/rollbar_test.go b/rollbar_test.go index 2ce1460..16b0f84 100644 --- a/rollbar_test.go +++ b/rollbar_test.go @@ -256,6 +256,67 @@ func TestFlattenValues(t *testing.T) { } } +func TestFilterFlatten(t *testing.T) { + values := map[string][]string{ + "password": {"one"}, + "ok": {"one", "two"}, + "access_token": {"one", "two"}, + "thing": {"foo", "bar"}, + "a": {"single"}, + "b": {"more", "than", "one"}, + } + + clean := filterFlatten(std.configuration.scrubFields, values, nil) + if clean["password"] != FILTERED { + t.Error("should filter password parameter") + } + + if clean["ok"] == FILTERED { + t.Error("should keep ok parameter") + } + + if len(clean["ok"].([]string)) != 2 { + t.Error("should not flatten ok parameter") + } + + if clean["access_token"] != FILTERED { + t.Error("should filter access_token parameter") + } + + special := map[string]struct{}{ + "thing": struct{}{}, + } + + clean2 := filterFlatten(std.configuration.scrubFields, values, special) + if clean2["password"] != FILTERED { + t.Error("should filter password parameter") + } + + if clean2["ok"] == FILTERED { + t.Error("should keep ok parameter") + } + + if len(clean2["ok"].([]string)) != 2 { + t.Error("should not flatten ok parameter") + } + + if clean2["access_token"] != FILTERED { + t.Error("should filter access_token parameter") + } + + if clean2["thing"] != "foo" { + t.Error("should force flatten a special key") + } + + if clean2["a"].(string) != "single" { + t.Error("should flatten single parameter to string") + } + + if len(clean2["b"].([]string)) != 3 { + t.Error("should leave multiple parametres as []string") + } +} + type cs struct { error cause error diff --git a/transforms.go b/transforms.go index ef039d9..950a023 100644 --- a/transforms.go +++ b/transforms.go @@ -80,22 +80,51 @@ func addErrorToBody(configuration configuration, body map[string]interface{}, er func requestDetails(configuration configuration, r *http.Request) map[string]interface{} { cleanQuery := filterParams(configuration.scrubFields, r.URL.Query()) + specialHeaders := map[string]struct{}{ + "Content-Type": struct{}{}, + } return map[string]interface{}{ "url": r.URL.String(), "method": r.Method, - "headers": flattenValues(filterParams(configuration.scrubHeaders, r.Header)), + "headers": filterFlatten(configuration.scrubHeaders, r.Header, specialHeaders), // GET params "query_string": url.Values(cleanQuery).Encode(), "GET": flattenValues(cleanQuery), // POST / PUT params - "POST": flattenValues(filterParams(configuration.scrubFields, r.Form)), + "POST": filterFlatten(configuration.scrubFields, r.Form, nil), "user_ip": filterIp(r.RemoteAddr, configuration.captureIp), } } +// filterFlatten filters sensitive information like passwords from being sent to Rollbar, and +// also lifts any values with length one up to be a standalone string. The optional specialKeys map +// will force strings that exist in that map and also in values to have a single string value in the +// resulting map by taking the first element in the list of strings if there are more than one. +// This is essentially the same as the composition of filterParams and filterValues, plus the bit +// extra about the special keys. The composition would range of the values twice when we really only +// need to do it once, so I decided to combine them as the result is still quite easy to follow. +// We keep the other two so that we can use url.Values.Encode on the filtered query params and not +// run the filtering twice for the query. +func filterFlatten(pattern *regexp.Regexp, values map[string][]string, specialKeys map[string]struct{}) map[string]interface{} { + result := make(map[string]interface{}) + + for k, v := range values { + switch _, special := specialKeys[k]; { + case pattern.Match([]byte(k)): + result[k] = FILTERED + case special || len(v) == 1: + result[k] = v[0] + default: + result[k] = v + } + } + + return result +} + // filterParams filters sensitive information like passwords from being sent to // Rollbar. func filterParams(pattern *regexp.Regexp, values map[string][]string) map[string][]string { From 8158322122093b76a4348df85fd6f89359be79da Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Thu, 18 Oct 2018 12:19:03 -0700 Subject: [PATCH 2/2] add a test for a request with multiple content-type values --- rollbar_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/rollbar_test.go b/rollbar_test.go index 16b0f84..cc195c8 100644 --- a/rollbar_test.go +++ b/rollbar_test.go @@ -219,6 +219,46 @@ func TestErrorRequest(t *testing.T) { } } +func TestErrorRequestHeaders(t *testing.T) { + r, _ := http.NewRequest("GET", "http://foo.com/somethere?param1=true", nil) + r.RemoteAddr = "1.1.1.1:123" + r.Header.Add("Content-Type", "application/json") + r.Header.Add("Content-Type", "application/x-json") + r.Header.Add("X-Foo-Bar", "baz") + r.Header.Add("X-Mult", "a") + r.Header.Add("X-Mult", "b") + + object := std.requestDetails(r) + + if object["url"] != "http://foo.com/somethere?param1=true" { + t.Errorf("wrong url, got %v", object["url"]) + } + + if object["method"] != "GET" { + t.Errorf("wrong method, got %v", object["method"]) + } + + if object["query_string"] != "param1=true" { + t.Errorf("wrong id, got %v", object["query_string"]) + } + + headers := object["headers"].(map[string]interface{}) + if headers["Content-Type"].(string) != "application/json" { + t.Errorf("expected single string value for Content-Type, got %v", headers["Content-Type"]) + } + if headers["X-Foo-Bar"].(string) != "baz" { + t.Errorf("expected single string value for X-Foo-Bar, got %v", headers["X-Foo-Bar"]) + } + if len(headers["X-Mult"].([]string)) != 2 { + t.Errorf("expected X-Mult to have two string values, got %v", headers["X-Mult"]) + } + + multHeaders := headers["X-Mult"].([]string) + if multHeaders[0] != "a" || multHeaders[1] != "b" { + t.Errorf("expected multiple string values for X-Mult, got %v", headers["X-Mult"]) + } +} + func TestFilterParams(t *testing.T) { values := map[string][]string{ "password": {"one"},