From 28cf35f684fb7a24c3bccdf52045a5012ce8ccd9 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Tue, 16 Jan 2024 12:15:28 +0100 Subject: [PATCH 01/10] 5458 task: [KIUWAN] Resolver golium vulnerabilities --- steps/common/sanitize.go | 26 +++++++++++++++++ steps/common/steps.go | 9 +++++- steps/dns/session.go | 5 ++-- steps/http/session.go | 6 ++-- test/acceptance/features/http/request.feature | 28 +++++++++++++++++-- 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 steps/common/sanitize.go diff --git a/steps/common/sanitize.go b/steps/common/sanitize.go new file mode 100644 index 00000000..d3c8fd49 --- /dev/null +++ b/steps/common/sanitize.go @@ -0,0 +1,26 @@ +package common + +import ( + "net/url" + "regexp" +) + +// Neutralization for unwanted command injections in domain string +func NeutralizeDomain(input string) string { + pattern := "^(?:https?://)?(?:www.)?([^:/\n&=?¿\"!| %]+)" + regex := regexp.MustCompile(pattern) + return regex.FindString(input) +} + +// Neutralization HTTP parameter pollution. CWE:235 +func NeutralizeParamPollution(queryParams map[string][]string) string { + params := url.Values{} + for key, values := range queryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + return params.Encode() +} diff --git a/steps/common/steps.go b/steps/common/steps.go index df76fa02..2b247e51 100755 --- a/steps/common/steps.go +++ b/steps/common/steps.go @@ -79,7 +79,14 @@ func (cs Steps) InitializeSteps(ctx context.Context, scenCtx *godog.ScenarioCont return nil } domain := golium.ValueAsString(ctx, domainParam) - command := fmt.Sprintf("ping -c 1 %s | head -1 | grep -oe '[0-9]*\\.[0-9]*\\.[0-9]*\\.[0-9]*'", domain) + domainN := NeutralizeDomain(domain) + + uri, err := url.Parse(domainN) + if err != nil { + return fmt.Errorf("failed parsing domain '%s': %w", domainN, err) + } + + command := fmt.Sprintf("ping -c 1 %s | head -1 | grep -oe '[0-9]*\\.[0-9]*\\.[0-9]*\\.[0-9]*'", uri) cmd := exec.Command("/bin/sh", "-c", command) stdoutStderr, err := cmd.CombinedOutput() if err != nil { diff --git a/steps/dns/session.go b/steps/dns/session.go index cba8cc96..5cfdab69 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -26,6 +26,7 @@ import ( "time" "github.com/AdguardTeam/dnsproxy/upstream" + "github.com/TelefonicaTC2Tech/golium/steps/common" "github.com/google/uuid" "github.com/miekg/dns" ) @@ -152,8 +153,8 @@ func (s *Session) SendDoHQuery( if errParse != nil { return err } - params := url.Values(s.DoHQueryParams) - u.RawQuery = params.Encode() + // Resolves HTTP parameter pollution. CWE:235 + u.RawQuery = common.NeutralizeParamPollution(s.DoHQueryParams) request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data)) if err != nil { return err diff --git a/steps/http/session.go b/steps/http/session.go index a2f971cc..5f0cffee 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -30,6 +30,7 @@ import ( "time" "github.com/TelefonicaTC2Tech/golium" + "github.com/TelefonicaTC2Tech/golium/steps/common" "github.com/TelefonicaTC2Tech/golium/steps/http/model" "github.com/TelefonicaTC2Tech/golium/steps/http/schema" "github.com/cucumber/godog" @@ -83,8 +84,9 @@ func (s *Session) URL() (*url.URL, error) { // * - Docs: https://pkg.go.dev/path#Join // */ - params := url.Values(s.Request.QueryParams) - u.RawQuery = params.Encode() + // Resolves HTTP parameter pollution. CWE:235 + u.RawQuery = common.NeutralizeParamPollution(s.Request.QueryParams) + return u, nil } diff --git a/test/acceptance/features/http/request.feature b/test/acceptance/features/http/request.feature index 0763558f..8e0603dc 100644 --- a/test/acceptance/features/http/request.feature +++ b/test/acceptance/features/http/request.feature @@ -29,6 +29,30 @@ Feature: HTTP Request Senders | id | 8 | Then the HTTP status code must be "200" + @http @request + Scenario: Send a GET request with parameter pollution. Duplicated keys + When I send a "GET" request to "posts" endpoint with query params + | field | value | + | userId | 1 | + | id | 1 | + | userId | 2 | + Then the HTTP status code must be "200" + And the HTTP response body must have the JSON properties + | property | value | + | # | [NUMBER:1] | + | 0.userId | [NUMBER:1] | + | 0.id | [NUMBER:1] | + + @http @request + Scenario: Send a GET request with parameter pollution. Encode query parameters + When I send a "GET" request to "posts" endpoint with query params + | field | value | + | id | 1&userId=1 | + Then the HTTP status code must be "200" + And the HTTP response body must have the JSON properties + | property | value | + | # | [NUMBER:0] | + @http @request Scenario Outline: Send a GET request with single filter When I send a "GET" request to "posts" endpoint with "" filters @@ -36,7 +60,7 @@ Feature: HTTP Request Senders And the HTTP response body must have the JSON properties | property | value | | # | | - Examples: + Examples: | filters | filtered_values | | userId=1 | [NUMBER:10] | | id=8 | [NUMBER:1] | @@ -48,7 +72,7 @@ Feature: HTTP Request Senders And the HTTP response body must have the JSON properties | property | value | | # | | - Examples: + Examples: | filters | filtered_values | | id=8&userId=1 | [NUMBER:1] | From e22918e15513ab165f0b99678dbad43b2fccaf83 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Tue, 16 Jan 2024 13:08:33 +0100 Subject: [PATCH 02/10] Change neutralize methods to private --- steps/common/sanitize.go | 26 -------------------------- steps/common/steps.go | 24 +++++++++++++++++------- steps/dns/session.go | 18 +++++++++++++++--- steps/http/session.go | 17 ++++++++++++++--- 4 files changed, 46 insertions(+), 39 deletions(-) delete mode 100644 steps/common/sanitize.go diff --git a/steps/common/sanitize.go b/steps/common/sanitize.go deleted file mode 100644 index d3c8fd49..00000000 --- a/steps/common/sanitize.go +++ /dev/null @@ -1,26 +0,0 @@ -package common - -import ( - "net/url" - "regexp" -) - -// Neutralization for unwanted command injections in domain string -func NeutralizeDomain(input string) string { - pattern := "^(?:https?://)?(?:www.)?([^:/\n&=?¿\"!| %]+)" - regex := regexp.MustCompile(pattern) - return regex.FindString(input) -} - -// Neutralization HTTP parameter pollution. CWE:235 -func NeutralizeParamPollution(queryParams map[string][]string) string { - params := url.Values{} - for key, values := range queryParams { - for _, value := range values { - if !params.Has(key) { - params.Add(key, value) - } - } - } - return params.Encode() -} diff --git a/steps/common/steps.go b/steps/common/steps.go index 2b247e51..0e23e64e 100755 --- a/steps/common/steps.go +++ b/steps/common/steps.go @@ -21,6 +21,7 @@ import ( "net" "net/url" "os/exec" + "regexp" "strings" "time" @@ -79,14 +80,9 @@ func (cs Steps) InitializeSteps(ctx context.Context, scenCtx *godog.ScenarioCont return nil } domain := golium.ValueAsString(ctx, domainParam) - domainN := NeutralizeDomain(domain) + domainN := neutralize(domain) - uri, err := url.Parse(domainN) - if err != nil { - return fmt.Errorf("failed parsing domain '%s': %w", domainN, err) - } - - command := fmt.Sprintf("ping -c 1 %s | head -1 | grep -oe '[0-9]*\\.[0-9]*\\.[0-9]*\\.[0-9]*'", uri) + command := fmt.Sprintf("ping -c 1 %s | head -1 | grep -oe '[0-9]*\\.[0-9]*\\.[0-9]*\\.[0-9]*'", domainN) cmd := exec.Command("/bin/sh", "-c", command) stdoutStderr, err := cmd.CombinedOutput() if err != nil { @@ -185,3 +181,17 @@ func getLocalIP(ctx context.Context, key string, ipVersion IPVersion) error { golium.GetContext(ctx).Put(golium.ValueAsString(ctx, key), localAddress.IP.String()) return nil } + +// Neutralization for unwanted command injections in domain string +func neutralize(input string) string { + pattern := "^(?:https?://)?(?:www.)?([^:/\n&=?¿\"!| %]+)" + regex := regexp.MustCompile(pattern) + domainN := regex.FindString(input) + + uri, err := url.Parse(domainN) + if err != nil { + return "" + } + + return uri.String() +} diff --git a/steps/dns/session.go b/steps/dns/session.go index 5cfdab69..b2f29c82 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -26,7 +26,6 @@ import ( "time" "github.com/AdguardTeam/dnsproxy/upstream" - "github.com/TelefonicaTC2Tech/golium/steps/common" "github.com/google/uuid" "github.com/miekg/dns" ) @@ -153,8 +152,8 @@ func (s *Session) SendDoHQuery( if errParse != nil { return err } - // Resolves HTTP parameter pollution. CWE:235 - u.RawQuery = common.NeutralizeParamPollution(s.DoHQueryParams) + + u.RawQuery = neutralize(s.DoHQueryParams) request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data)) if err != nil { return err @@ -286,3 +285,16 @@ func (s *Session) ValidateResponseWithRecords( } return nil } + +// Neutralization HTTP parameter pollution. CWE:235 +func neutralize(queryParams map[string][]string) string { + params := url.Values{} + for key, values := range queryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + return params.Encode() +} diff --git a/steps/http/session.go b/steps/http/session.go index 5f0cffee..198a3ed2 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -30,7 +30,6 @@ import ( "time" "github.com/TelefonicaTC2Tech/golium" - "github.com/TelefonicaTC2Tech/golium/steps/common" "github.com/TelefonicaTC2Tech/golium/steps/http/model" "github.com/TelefonicaTC2Tech/golium/steps/http/schema" "github.com/cucumber/godog" @@ -84,8 +83,7 @@ func (s *Session) URL() (*url.URL, error) { // * - Docs: https://pkg.go.dev/path#Join // */ - // Resolves HTTP parameter pollution. CWE:235 - u.RawQuery = common.NeutralizeParamPollution(s.Request.QueryParams) + u.RawQuery = neutralize(s.Request.QueryParams) return u, nil } @@ -689,3 +687,16 @@ func (s *Session) GetURL(ctx context.Context) (string, error) { } return URL, nil } + +// Neutralization HTTP parameter pollution. CWE:235 +func neutralize(queryParams map[string][]string) string { + params := url.Values{} + for key, values := range queryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + return params.Encode() +} From b6fc400aabf064b8214b631073b46e167819cc9b Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Tue, 16 Jan 2024 13:33:21 +0100 Subject: [PATCH 03/10] Change neutralize name method to sanitize --- steps/dns/session.go | 6 +++--- steps/http/session.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/steps/dns/session.go b/steps/dns/session.go index b2f29c82..ca1f4004 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -153,7 +153,7 @@ func (s *Session) SendDoHQuery( return err } - u.RawQuery = neutralize(s.DoHQueryParams) + u.RawQuery = sanitize(s.DoHQueryParams) request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data)) if err != nil { return err @@ -286,8 +286,8 @@ func (s *Session) ValidateResponseWithRecords( return nil } -// Neutralization HTTP parameter pollution. CWE:235 -func neutralize(queryParams map[string][]string) string { +// Sanitize HTTP parameter pollution. CWE:235 +func sanitize(queryParams map[string][]string) string { params := url.Values{} for key, values := range queryParams { for _, value := range values { diff --git a/steps/http/session.go b/steps/http/session.go index 198a3ed2..ba002bb5 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -83,7 +83,7 @@ func (s *Session) URL() (*url.URL, error) { // * - Docs: https://pkg.go.dev/path#Join // */ - u.RawQuery = neutralize(s.Request.QueryParams) + u.RawQuery = sanitize(s.Request.QueryParams) return u, nil } @@ -688,8 +688,8 @@ func (s *Session) GetURL(ctx context.Context) (string, error) { return URL, nil } -// Neutralization HTTP parameter pollution. CWE:235 -func neutralize(queryParams map[string][]string) string { +// Sanitize HTTP parameter pollution. CWE:235 +func sanitize(queryParams map[string][]string) string { params := url.Values{} for key, values := range queryParams { for _, value := range values { From d08ea342ac89cf6b9d6cc911e5c2dad50668118b Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Tue, 16 Jan 2024 16:20:12 +0100 Subject: [PATCH 04/10] New way to neutralize and sanitize --- steps/common/steps.go | 13 ++++++++++--- steps/dns/session.go | 12 +++++++++++- steps/http/session.go | 3 ++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/steps/common/steps.go b/steps/common/steps.go index 0e23e64e..4481517a 100755 --- a/steps/common/steps.go +++ b/steps/common/steps.go @@ -80,10 +80,11 @@ func (cs Steps) InitializeSteps(ctx context.Context, scenCtx *godog.ScenarioCont return nil } domain := golium.ValueAsString(ctx, domainParam) - domainN := neutralize(domain) + domainN := neutralizeDomain(domain) command := fmt.Sprintf("ping -c 1 %s | head -1 | grep -oe '[0-9]*\\.[0-9]*\\.[0-9]*\\.[0-9]*'", domainN) - cmd := exec.Command("/bin/sh", "-c", command) + commandN := neutralize(command) + cmd := exec.Command("/bin/sh", "-c", commandN) stdoutStderr, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("error executing `%s` command %v", cmd, string(stdoutStderr)) @@ -95,6 +96,12 @@ func (cs Steps) InitializeSteps(ctx context.Context, scenCtx *godog.ScenarioCont return ctx } +func neutralize(p string) string { + p = strings.ReplaceAll(p, "\r", "") + p = strings.ReplaceAll(p, "\n", "") + return p +} + // StoreValueInContext stores a value in golium.Context using the key name. func StoreValueInContext(ctx context.Context, name, value string) error { golium.GetContext(ctx).Put(name, value) @@ -183,7 +190,7 @@ func getLocalIP(ctx context.Context, key string, ipVersion IPVersion) error { } // Neutralization for unwanted command injections in domain string -func neutralize(input string) string { +func neutralizeDomain(input string) string { pattern := "^(?:https?://)?(?:www.)?([^:/\n&=?¿\"!| %]+)" regex := regexp.MustCompile(pattern) domainN := regex.FindString(input) diff --git a/steps/dns/session.go b/steps/dns/session.go index ca1f4004..afaf1fcd 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -23,6 +23,7 @@ import ( "io" "net/http" "net/url" + "strings" "time" "github.com/AdguardTeam/dnsproxy/upstream" @@ -153,7 +154,10 @@ func (s *Session) SendDoHQuery( return err } - u.RawQuery = sanitize(s.DoHQueryParams) + rawQuery := sanitize(s.DoHQueryParams) + rawQueryN := neutralize(rawQuery) + u.RawQuery = rawQueryN + request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data)) if err != nil { return err @@ -298,3 +302,9 @@ func sanitize(queryParams map[string][]string) string { } return params.Encode() } + +func neutralize(p string) string { + p = strings.ReplaceAll(p, "\r", "") + p = strings.ReplaceAll(p, "\n", "") + return p +} diff --git a/steps/http/session.go b/steps/http/session.go index ba002bb5..29ab4902 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -83,7 +83,8 @@ func (s *Session) URL() (*url.URL, error) { // * - Docs: https://pkg.go.dev/path#Join // */ - u.RawQuery = sanitize(s.Request.QueryParams) + rawQueryN := sanitize(s.Request.QueryParams) + u.RawQuery = rawQueryN return u, nil } From 40512aeb40307e7994090296534aee9f1507c670 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Fri, 9 Feb 2024 10:38:47 +0100 Subject: [PATCH 05/10] modify the sanitize and neutralize declaration at the beginning of the file --- steps/common/steps.go | 40 ++++++++++++++++++++-------------------- steps/dns/session.go | 38 +++++++++++++++++++------------------- steps/http/session.go | 26 +++++++++++++------------- 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/steps/common/steps.go b/steps/common/steps.go index 4481517a..e1564560 100755 --- a/steps/common/steps.go +++ b/steps/common/steps.go @@ -30,6 +30,26 @@ import ( "github.com/google/uuid" ) +// Neutralization for unwanted command injections in domain string +func neutralizeDomain(input string) string { + pattern := "^(?:https?://)?(?:www.)?([^:/\n&=?¿\"!| %]+)" + regex := regexp.MustCompile(pattern) + domainN := regex.FindString(input) + + uri, err := url.Parse(domainN) + if err != nil { + return "" + } + + return uri.String() +} + +func neutralize(p string) string { + p = strings.ReplaceAll(p, "\r", "") + p = strings.ReplaceAll(p, "\n", "") + return p +} + // Steps to initialize common steps. type Steps struct { } @@ -96,12 +116,6 @@ func (cs Steps) InitializeSteps(ctx context.Context, scenCtx *godog.ScenarioCont return ctx } -func neutralize(p string) string { - p = strings.ReplaceAll(p, "\r", "") - p = strings.ReplaceAll(p, "\n", "") - return p -} - // StoreValueInContext stores a value in golium.Context using the key name. func StoreValueInContext(ctx context.Context, name, value string) error { golium.GetContext(ctx).Put(name, value) @@ -188,17 +202,3 @@ func getLocalIP(ctx context.Context, key string, ipVersion IPVersion) error { golium.GetContext(ctx).Put(golium.ValueAsString(ctx, key), localAddress.IP.String()) return nil } - -// Neutralization for unwanted command injections in domain string -func neutralizeDomain(input string) string { - pattern := "^(?:https?://)?(?:www.)?([^:/\n&=?¿\"!| %]+)" - regex := regexp.MustCompile(pattern) - domainN := regex.FindString(input) - - uri, err := url.Parse(domainN) - if err != nil { - return "" - } - - return uri.String() -} diff --git a/steps/dns/session.go b/steps/dns/session.go index afaf1fcd..8b6f2305 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -31,6 +31,25 @@ import ( "github.com/miekg/dns" ) +// Sanitize HTTP parameter pollution. CWE:235 +func sanitize(queryParams map[string][]string) string { + params := url.Values{} + for key, values := range queryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + return params.Encode() +} + +func neutralize(p string) string { + p = strings.ReplaceAll(p, "\r", "") + p = strings.ReplaceAll(p, "\n", "") + return p +} + // Session contains the information related to a DNS query and response. type Session struct { // Server is the address to the DNS server, including the server port (e.g. 8.8.8.8:53). @@ -289,22 +308,3 @@ func (s *Session) ValidateResponseWithRecords( } return nil } - -// Sanitize HTTP parameter pollution. CWE:235 -func sanitize(queryParams map[string][]string) string { - params := url.Values{} - for key, values := range queryParams { - for _, value := range values { - if !params.Has(key) { - params.Add(key, value) - } - } - } - return params.Encode() -} - -func neutralize(p string) string { - p = strings.ReplaceAll(p, "\r", "") - p = strings.ReplaceAll(p, "\n", "") - return p -} diff --git a/steps/http/session.go b/steps/http/session.go index 29ab4902..adfc50b4 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -49,6 +49,19 @@ const ( DefaultTestURL = "https://jsonplaceholder.typicode.com/" ) +// Sanitize HTTP parameter pollution. CWE:235 +func sanitize(queryParams map[string][]string) string { + params := url.Values{} + for key, values := range queryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + return params.Encode() +} + // Session contains the information of a HTTP session (request and response). type Session struct { Request model.Request @@ -688,16 +701,3 @@ func (s *Session) GetURL(ctx context.Context) (string, error) { } return URL, nil } - -// Sanitize HTTP parameter pollution. CWE:235 -func sanitize(queryParams map[string][]string) string { - params := url.Values{} - for key, values := range queryParams { - for _, value := range values { - if !params.Has(key) { - params.Add(key, value) - } - } - } - return params.Encode() -} From 561a7fbb3f8edac1c01f74dd5ef0c9e59cfa1f45 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Fri, 9 Feb 2024 13:38:51 +0100 Subject: [PATCH 06/10] neutralize all query params --- steps/http/session.go | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/steps/http/session.go b/steps/http/session.go index adfc50b4..e2932500 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -27,6 +27,7 @@ import ( "net/url" "path" "reflect" + "strings" "time" "github.com/TelefonicaTC2Tech/golium" @@ -49,17 +50,11 @@ const ( DefaultTestURL = "https://jsonplaceholder.typicode.com/" ) -// Sanitize HTTP parameter pollution. CWE:235 -func sanitize(queryParams map[string][]string) string { - params := url.Values{} - for key, values := range queryParams { - for _, value := range values { - if !params.Has(key) { - params.Add(key, value) - } - } - } - return params.Encode() +// Neutralize HTTP parameter pollution. CWE:235 +func neutralize(p string) string { + p = strings.ReplaceAll(p, "\r", "") + p = strings.ReplaceAll(p, "\n", "") + return p } // Session contains the information of a HTTP session (request and response). @@ -95,9 +90,17 @@ func (s *Session) URL() (*url.URL, error) { // * - Reference: https://forum.golangbridge.org/t/how-to-concatenate-paths-for-api-request/5791 // * - Docs: https://pkg.go.dev/path#Join // */ - - rawQueryN := sanitize(s.Request.QueryParams) - u.RawQuery = rawQueryN + params := url.Values{} + for key, values := range s.Request.QueryParams { + for _, value := range values { + if !params.Has(key) { + keyN := neutralize(key) + valueN := neutralize(value) + params.Add(keyN, valueN) + } + } + } + u.RawQuery = neutralize(params.Encode()) return u, nil } From cf65c82e319c39056c60ed4671e1d35890cf54d7 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Fri, 9 Feb 2024 14:21:34 +0100 Subject: [PATCH 07/10] change where neutralize is applied --- steps/http/session.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/steps/http/session.go b/steps/http/session.go index e2932500..ca2838bf 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -94,9 +94,7 @@ func (s *Session) URL() (*url.URL, error) { for key, values := range s.Request.QueryParams { for _, value := range values { if !params.Has(key) { - keyN := neutralize(key) - valueN := neutralize(value) - params.Add(keyN, valueN) + params.Add(key, value) } } } @@ -240,7 +238,7 @@ func (s *Session) SendHTTPRequest(ctx context.Context, method string) error { if s.Request.Headers != nil { hostHeaders, found := s.Request.Headers["Host"] if found && len(hostHeaders) > 0 { - req.Host = hostHeaders[0] + req.Host = neutralize(hostHeaders[0]) } } req.Header = s.Request.Headers From e9b216667421679f76fc10a282090c4f1f60a9c9 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Fri, 9 Feb 2024 14:46:10 +0100 Subject: [PATCH 08/10] another change to sanitize query params --- steps/http/session.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/steps/http/session.go b/steps/http/session.go index ca2838bf..f7157c17 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -51,7 +51,21 @@ const ( ) // Neutralize HTTP parameter pollution. CWE:235 -func neutralize(p string) string { +func neutralize(queryParams map[string][]string) string { + params := url.Values{} + for key, values := range queryParams { + for _, value := range values { + if !params.Has(key) { + keyS := sanitize(key) + valueS := sanitize(value) + params.Add(keyS, valueS) + } + } + } + return params.Encode() +} + +func sanitize(p string) string { p = strings.ReplaceAll(p, "\r", "") p = strings.ReplaceAll(p, "\n", "") return p @@ -90,15 +104,7 @@ func (s *Session) URL() (*url.URL, error) { // * - Reference: https://forum.golangbridge.org/t/how-to-concatenate-paths-for-api-request/5791 // * - Docs: https://pkg.go.dev/path#Join // */ - params := url.Values{} - for key, values := range s.Request.QueryParams { - for _, value := range values { - if !params.Has(key) { - params.Add(key, value) - } - } - } - u.RawQuery = neutralize(params.Encode()) + u.RawQuery = neutralize(s.Request.QueryParams) return u, nil } @@ -238,7 +244,7 @@ func (s *Session) SendHTTPRequest(ctx context.Context, method string) error { if s.Request.Headers != nil { hostHeaders, found := s.Request.Headers["Host"] if found && len(hostHeaders) > 0 { - req.Host = neutralize(hostHeaders[0]) + req.Host = sanitize(hostHeaders[0]) } } req.Header = s.Request.Headers From b763299721a89fdd2da3c174c3efea0d3578818d Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Mon, 12 Feb 2024 16:32:46 +0100 Subject: [PATCH 09/10] Another change --- steps/common/steps.go | 3 +-- steps/dns/session.go | 26 ++++++++++---------------- steps/http/session.go | 28 +++++++++++----------------- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/steps/common/steps.go b/steps/common/steps.go index e1564560..23ff9c6f 100755 --- a/steps/common/steps.go +++ b/steps/common/steps.go @@ -103,8 +103,7 @@ func (cs Steps) InitializeSteps(ctx context.Context, scenCtx *godog.ScenarioCont domainN := neutralizeDomain(domain) command := fmt.Sprintf("ping -c 1 %s | head -1 | grep -oe '[0-9]*\\.[0-9]*\\.[0-9]*\\.[0-9]*'", domainN) - commandN := neutralize(command) - cmd := exec.Command("/bin/sh", "-c", commandN) + cmd := exec.Command("/bin/sh", "-c", neutralize(command)) stdoutStderr, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("error executing `%s` command %v", cmd, string(stdoutStderr)) diff --git a/steps/dns/session.go b/steps/dns/session.go index 8b6f2305..08c17c32 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -31,19 +31,7 @@ import ( "github.com/miekg/dns" ) -// Sanitize HTTP parameter pollution. CWE:235 -func sanitize(queryParams map[string][]string) string { - params := url.Values{} - for key, values := range queryParams { - for _, value := range values { - if !params.Has(key) { - params.Add(key, value) - } - } - } - return params.Encode() -} - +// Neutralize HTTP parameter pollution. CWE:235 func neutralize(p string) string { p = strings.ReplaceAll(p, "\r", "") p = strings.ReplaceAll(p, "\n", "") @@ -173,9 +161,15 @@ func (s *Session) SendDoHQuery( return err } - rawQuery := sanitize(s.DoHQueryParams) - rawQueryN := neutralize(rawQuery) - u.RawQuery = rawQueryN + params := url.Values{} + for key, values := range s.DoHQueryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + u.RawQuery = neutralize(params.Encode()) request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data)) if err != nil { diff --git a/steps/http/session.go b/steps/http/session.go index f7157c17..ca2838bf 100755 --- a/steps/http/session.go +++ b/steps/http/session.go @@ -51,21 +51,7 @@ const ( ) // Neutralize HTTP parameter pollution. CWE:235 -func neutralize(queryParams map[string][]string) string { - params := url.Values{} - for key, values := range queryParams { - for _, value := range values { - if !params.Has(key) { - keyS := sanitize(key) - valueS := sanitize(value) - params.Add(keyS, valueS) - } - } - } - return params.Encode() -} - -func sanitize(p string) string { +func neutralize(p string) string { p = strings.ReplaceAll(p, "\r", "") p = strings.ReplaceAll(p, "\n", "") return p @@ -104,7 +90,15 @@ func (s *Session) URL() (*url.URL, error) { // * - Reference: https://forum.golangbridge.org/t/how-to-concatenate-paths-for-api-request/5791 // * - Docs: https://pkg.go.dev/path#Join // */ - u.RawQuery = neutralize(s.Request.QueryParams) + params := url.Values{} + for key, values := range s.Request.QueryParams { + for _, value := range values { + if !params.Has(key) { + params.Add(key, value) + } + } + } + u.RawQuery = neutralize(params.Encode()) return u, nil } @@ -244,7 +238,7 @@ func (s *Session) SendHTTPRequest(ctx context.Context, method string) error { if s.Request.Headers != nil { hostHeaders, found := s.Request.Headers["Host"] if found && len(hostHeaders) > 0 { - req.Host = sanitize(hostHeaders[0]) + req.Host = neutralize(hostHeaders[0]) } } req.Header = s.Request.Headers From 68c0a835927e9bca9e0ce689a85f2aca0d460881 Mon Sep 17 00:00:00 2001 From: ruben-garciad Date: Tue, 13 Feb 2024 11:42:01 +0100 Subject: [PATCH 10/10] refactor in SendDoHQuery --- steps/dns/session.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/steps/dns/session.go b/steps/dns/session.go index 08c17c32..8da01e77 100644 --- a/steps/dns/session.go +++ b/steps/dns/session.go @@ -146,15 +146,14 @@ func (s *Session) SendDoHQuery( Transport: tr, } var request *http.Request + var urlStr string + var bodyRequest io.Reader switch method { case "GET": dq := base64.RawURLEncoding.EncodeToString(data) - urlStr := fmt.Sprintf("%s?dns=%s", s.Server, dq) - request, err = http.NewRequest("GET", urlStr, http.NoBody) - if err != nil { - return err - } + urlStr = fmt.Sprintf("%s?dns=%s", s.Server, dq) + bodyRequest = http.NoBody case "POST": u, errParse := url.Parse(s.Server) if errParse != nil { @@ -170,15 +169,16 @@ func (s *Session) SendDoHQuery( } } u.RawQuery = neutralize(params.Encode()) - - request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data)) - if err != nil { - return err - } + urlStr = u.String() + bodyRequest = bytes.NewReader(data) default: return fmt.Errorf("unsupported method. %s", method) } + request, err = http.NewRequest(method, neutralize(urlStr), bodyRequest) + if err != nil { + return err + } request.Header.Set("Content-Type", "application/dns-message") response, err := client.Do(request) if err != nil { @@ -189,14 +189,14 @@ func (s *Session) SendDoHQuery( return fmt.Errorf("error in Content-Type Header. Value: %s, Expected: %s", response.Header.Get("Content-Type"), "application/dns-message") } - body, err := io.ReadAll(response.Body) + bodyResponse, err := io.ReadAll(response.Body) if err != nil { return fmt.Errorf("error reading the response body. %s", err) } response.Body.Close() // Get the response body and Unpack it to convert from a DNS wireformat dnsResp := new(dns.Msg) - err = dnsResp.Unpack(body) + err = dnsResp.Unpack(bodyResponse) if err != nil { return fmt.Errorf("error unpacking body. %s", err) }