Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5458 task: [KIUWAN] Resolve golium vulnerabilities #123

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
27 changes: 25 additions & 2 deletions steps/common/steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"net"
"net/url"
"os/exec"
"regexp"
"strings"
"time"

Expand All @@ -29,6 +30,26 @@
"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 {
}
Expand Down Expand Up @@ -79,8 +100,10 @@
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)
cmd := exec.Command("/bin/sh", "-c", command)
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", neutralize(command))

Check failure on line 106 in steps/common/steps.go

View workflow job for this annotation

GitHub Actions / lint

G204: Subprocess launched with a potential tainted input or cmd arguments (gosec)

Check failure on line 106 in steps/common/steps.go

View workflow job for this annotation

GitHub Actions / lint

G204: Subprocess launched with a potential tainted input or cmd arguments (gosec)
stdoutStderr, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("error executing `%s` command %v", cmd, string(stdoutStderr))
Expand Down
41 changes: 29 additions & 12 deletions steps/dns/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,21 @@ import (
"io"
"net/http"
"net/url"
"strings"
"time"

"github.com/AdguardTeam/dnsproxy/upstream"
"github.com/google/uuid"
"github.com/miekg/dns"
)

// 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 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).
Expand Down Expand Up @@ -138,30 +146,39 @@ 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 {
return err
}
params := url.Values(s.DoHQueryParams)
u.RawQuery = params.Encode()
request, err = http.NewRequest("POST", u.String(), bytes.NewReader(data))
if err != nil {
return err

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())
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 {
Expand All @@ -172,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)
}
Expand Down
21 changes: 18 additions & 3 deletions steps/http/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"net/url"
"path"
"reflect"
"strings"
"time"

"github.com/TelefonicaTC2Tech/golium"
Expand All @@ -49,6 +50,13 @@ const (
DefaultTestURL = "https://jsonplaceholder.typicode.com/"
)

// 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).
type Session struct {
Request model.Request
Expand Down Expand Up @@ -82,9 +90,16 @@ 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())

params := url.Values(s.Request.QueryParams)
u.RawQuery = params.Encode()
return u, nil
}

Expand Down Expand Up @@ -223,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
Expand Down
28 changes: 26 additions & 2 deletions test/acceptance/features/http/request.feature
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,38 @@ 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>" filters
Then the HTTP status code must be "200"
And the HTTP response body must have the JSON properties
| property | value |
| # | <filtered_values> |
Examples:
Examples:
| filters | filtered_values |
| userId=1 | [NUMBER:10] |
| id=8 | [NUMBER:1] |
Expand All @@ -48,7 +72,7 @@ Feature: HTTP Request Senders
And the HTTP response body must have the JSON properties
| property | value |
| # | <filtered_values> |
Examples:
Examples:
| filters | filtered_values |
| id=8&userId=1 | [NUMBER:1] |

Expand Down
Loading