From 72e5ed1ed50900eefc6cea6ce071b392e55b5421 Mon Sep 17 00:00:00 2001 From: Tarun Koyalwar Date: Mon, 6 Feb 2023 21:13:34 +0530 Subject: [PATCH 1/3] fix double url encoding in path --- go.mod | 2 +- go.sum | 12 ++++-------- request.go | 12 ++++++++++++ request_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index de4c8e4..bf0b8c3 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( github.com/Mzack9999/go-http-digest-auth-client v0.6.1-0.20220414142836-eb8883508809 - github.com/projectdiscovery/utils v0.0.4-0.20230117135930-7371ae6a739d + github.com/projectdiscovery/utils v0.0.7 golang.org/x/net v0.5.0 ) diff --git a/go.sum b/go.sum index 1752b74..a79416f 100644 --- a/go.sum +++ b/go.sum @@ -8,14 +8,10 @@ github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl github.com/microcosm-cc/bluemonday v1.0.21 h1:dNH3e4PSyE4vNX+KlRGHT5KrSvjeUkoNPwEORjffHJg= github.com/microcosm-cc/bluemonday v1.0.21/go.mod h1:ytNkv4RrDrLJ2pqlsSI46O6IVXmZOBBD4SaJyDwwTkM= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/projectdiscovery/utils v0.0.4-0.20221201124851-f8524345b6d3 h1:sOvfN3xHLiBMb6GJ3yDxBmPnN0dh3xllaQXQYo7CFUo= -github.com/projectdiscovery/utils v0.0.4-0.20221201124851-f8524345b6d3/go.mod h1:PCwA5YuCYWPgHaGiZmr53/SA9iGQmAnw7DSHuhr8VPQ= -github.com/projectdiscovery/utils v0.0.4-0.20230117121210-1eaffe0d0834 h1:ehoX21rVDm+i7/o8OpTTtDdbesHshF0AD13gbc21wBA= -github.com/projectdiscovery/utils v0.0.4-0.20230117121210-1eaffe0d0834/go.mod h1:PCwA5YuCYWPgHaGiZmr53/SA9iGQmAnw7DSHuhr8VPQ= -github.com/projectdiscovery/utils v0.0.4-0.20230117132455-e51a5b2e562c h1:+iHkNvGP/1Cbq6lo8htaQZd3fWch8E9OKD0xTUwS+Zo= -github.com/projectdiscovery/utils v0.0.4-0.20230117132455-e51a5b2e562c/go.mod h1:PCwA5YuCYWPgHaGiZmr53/SA9iGQmAnw7DSHuhr8VPQ= -github.com/projectdiscovery/utils v0.0.4-0.20230117135930-7371ae6a739d h1:iB/n2/NL4oh1IaEcqX6pBxj0WHfYN7finzNOKVNVISM= -github.com/projectdiscovery/utils v0.0.4-0.20230117135930-7371ae6a739d/go.mod h1:PCwA5YuCYWPgHaGiZmr53/SA9iGQmAnw7DSHuhr8VPQ= +github.com/projectdiscovery/utils v0.0.7 h1:jqDuZedy3t66o6ejQUXjgNWbyAHqiBqLAUDkst9DA2M= +github.com/projectdiscovery/utils v0.0.7/go.mod h1:PCwA5YuCYWPgHaGiZmr53/SA9iGQmAnw7DSHuhr8VPQ= +github.com/projectdiscovery/utils v0.0.8-0.20230206142604-469c07cf5050 h1:PwtYD40LMJag5jpB3F2bi1y4tLAMUPIeuWO37txfbOI= +github.com/projectdiscovery/utils v0.0.8-0.20230206142604-469c07cf5050/go.mod h1:PCwA5YuCYWPgHaGiZmr53/SA9iGQmAnw7DSHuhr8VPQ= github.com/saintfish/chardet v0.0.0-20120816061221-3af4cd4741ca h1:NugYot0LIVPxTvN8n+Kvkn6TrbMyxQiuvKdEwFdR9vI= github.com/saintfish/chardet v0.0.0-20120816061221-3af4cd4741ca/go.mod h1:uugorj2VCxiV1x+LzaIdVa9b4S4qGAcH6cbhh4qVxOU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= diff --git a/request.go b/request.go index 6083f3f..af0f924 100644 --- a/request.go +++ b/request.go @@ -107,6 +107,18 @@ func (r *Request) Update() { updateScheme(r.URL.URL) } +// Prepares request (applies hot patch if any. Ex: Path Unescape to prevent double url encoding) +// calling multiple times may have unexpected results unlike Update() method +func (r *Request) Prepare() { + // hot patch to avoid url path encoding issues + // by default we decode encoded/escaped path and internally http.Request encodes them again + // this avoid double url encoding (or reencoding or path) + if rawPath, err := url.QueryUnescape(r.URL.Path); err == nil { + r.URL.Path = rawPath + } + r.Update() +} + // SetURL updates request url (i.e http.Request.URL) with given url func (r *Request) SetURL(u *urlutil.URL) { r.URL = u diff --git a/request_test.go b/request_test.go index 1177dcc..b6dd238 100644 --- a/request_test.go +++ b/request_test.go @@ -1,7 +1,10 @@ package retryablehttp_test import ( + "bufio" + "bytes" "os" + "strings" "testing" "github.com/projectdiscovery/retryablehttp-go" @@ -35,3 +38,51 @@ func TestRequestUrls(t *testing.T) { } } } + +func TestEncodedPaths(t *testing.T) { + + // test this on all valid crlf payloads + payloads := []string{"%00", "%0a", "%0a%20", "%0d", "%0d%09", "%0d%0a", "%0d%0a%09", "%0d%0a%20", "%0d%20", "%20", "%20%0a", "%20%0d", "%20%0d%0a", "%23%0a", "%23%0a%20", "%23%0d", "%23%0d%0a", "%23%0a", "%25%30", "%25%30%61", "%2e%2e%2f%0d%0a", "%2f%2e%2e%0d%0a", "%2f..%0d%0a", "%3f", "%3f%0a", "%3f%0d", "%3f%0d%0a", "%e5%98%8a%e5%98%8d", "%e5%98%8a%e5%98%8d%0a", "%e5%98%8a%e5%98%8d%0d", "%e5%98%8a%e5%98%8d%0d%0a", "%e5%98%8a%e5%98%8d%e5%98%8a%e5%98%8d"} + + // create url using below data and payload + suffix := "/path?param=true" + + for _, v := range payloads { + exURL := "https://scanme.sh/" + v + suffix + req, err := retryablehttp.NewRequest("GET", exURL, nil) + if err != nil { + t.Fatalf("got %v with payload %v", err.Error(), v) + } + + req.Prepare() + bin, err := req.Dump() + if err != nil { + t.Errorf("failed to dump request body for payload %v got %v", v, err) + } + + relPath := getPathFromRaw(bin) + payload := strings.TrimSuffix(relPath, suffix) + payload = strings.TrimPrefix(payload, "/") + + if v != payload { + t.Errorf("something went wrong expected `%v` in outgoing request but got-----\n%v\n------", v, string(bin)) + } + } +} + +func getPathFromRaw(bin []byte) (relpath string) { + buff := bufio.NewReader(bytes.NewReader(bin)) +readline: + line, err := buff.ReadString('\n') + if err != nil { + return + } + if strings.Contains(line, "HTTP/1.1") { + parts := strings.Split(line, " ") + if len(parts) == 3 { + relpath = parts[1] + return + } + } + goto readline +} From acb223323d89513d1fafd6f0fe52a56ac5881031 Mon Sep 17 00:00:00 2001 From: Tarun Koyalwar Date: Wed, 8 Feb 2023 19:01:57 +0530 Subject: [PATCH 2/3] add docs & other minor improvements --- .github/workflows/build-test.yml | 6 +++- README.md | 57 +++++++++++++++++--------------- examples/main.go | 25 ++++++++++++++ request.go | 44 +++++++++++------------- 4 files changed, 81 insertions(+), 51 deletions(-) create mode 100644 examples/main.go diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 3bd35ca..ca5360b 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -21,4 +21,8 @@ jobs: uses: actions/checkout@v3 - name: Test - run: go test ./... \ No newline at end of file + run: go test ./... + + - name: Run Example + run: go run . + working-directory: examples/ \ No newline at end of file diff --git a/README.md b/README.md index 8b54585..9664234 100644 --- a/README.md +++ b/README.md @@ -4,30 +4,35 @@ Heavily inspired from [https://github.com/hashicorp/go-retryablehttp](https://gi ### Usage -```go -package main - -import ( - "fmt" - "io/ioutil" - - "github.com/projectdiscovery/retryablehttp-go" -) - -func main() { - opts := retryablehttp.DefaultOptionsSpraying - // opts := retryablehttp.DefaultOptionsSingle // use single options for single host - client := retryablehttp.NewClient(opts) - resp, err := client.Get("https://example.com") - if err != nil { - panic(err) - } - defer resp.Body.Close() - - data, err := io.ReadAll(resp.Body) - if err != nil { - panic(err) - } - fmt.Printf("Data: %v\n", string(data)) -} +Example of using `retryablehttp` in Go Code is available in [examples](examples/) folder +Examples of using Nuclei From Go Code to run templates on targets are provided in the examples folder. + + + + +### url encoding and parsing issues + +`retryablehttp.Request` by default handles some [url encoding and parameters issues](https://github.com/projectdiscovery/utils/blob/main/url/README.md). since `http.Request` internally uses `url.Parse()` to parse url specified in request it creates some inconsistencies for below urls and other non-RFC compilant urls + +``` +// below urls are either normalized or returns error when used in `http.NewRequest()` +https://scanme.sh/%invalid +https://scanme.sh/w%0d%2e/ +scanme.sh/with/path?some'param=`'+OR+ORDER+BY+1-- ``` +All above mentioned cases are handled internally in `retryablehttp`. + + +### request with unsafe urls + +`retryablehttp` allows creating requests with unsafe urls but requires some extra steps if `path` of url contains encoded characters (ex: `/%invalid/path`). + +- `Request.Prepare()` method should be called before request is executed and this applies a quick fix to avoid double url encoding (ex: `%e5` => `%25e5`) + +Note: this is a optional feature and only required if we want to allow unsafe urls. If `Request.Prepare()` is not called it follows the standard behaviour. + +### Note +It is not recommended to update `url.URL` instance of `Request` once a new request is created (ex `req.URL.Path = xyz`) due to internal logic or urls. +In any case if it is not possible to follow above point due to some reason helper methods are available to reflect such changes + +- `Request.Update()` commits any changes made to query parameters (ex: `Request.URL.Query().Add(x,y)`) diff --git a/examples/main.go b/examples/main.go new file mode 100644 index 0000000..104ccd3 --- /dev/null +++ b/examples/main.go @@ -0,0 +1,25 @@ +package main + +import ( + "fmt" + "io" + + "github.com/projectdiscovery/retryablehttp-go" +) + +func main() { + opts := retryablehttp.DefaultOptionsSpraying + // opts := retryablehttp.DefaultOptionsSingle // use single options for single host + client := retryablehttp.NewClient(opts) + resp, err := client.Get("https://scanme.sh") + if err != nil { + panic(err) + } + defer resp.Body.Close() + + data, err := io.ReadAll(resp.Body) + if err != nil { + panic(err) + } + fmt.Printf("Data: %v\n", string(data)) +} diff --git a/request.go b/request.go index af0f924..863d095 100644 --- a/request.go +++ b/request.go @@ -227,23 +227,28 @@ func FromRequestWithTrace(r *http.Request) (*Request, error) { } // NewRequest creates a new wrapped request. -func NewRequest(method, url string, body interface{}) (*Request, error) { +func NewRequestFromURL(method string, urlx *urlutil.URL, body interface{}) (*Request, error) { + return NewRequestFromURLWithContext(context.Background(), method, urlx, body) +} + +// NewRequestWithContext creates a new wrapped request with context +func NewRequestFromURLWithContext(ctx context.Context, method string, urlx *urlutil.URL, body interface{}) (*Request, error) { bodyReader, contentLength, err := getReusableBodyandContentLength(body) if err != nil { return nil, err } - urlx, err := urlutil.Parse(url) - if err != nil { - return nil, err - } - httpReq, err := http.NewRequest(method, url, nil) + // we provide a dummy url at start and then replace url instance directly + // because `http.NewRequest()` internally parses using `url.Parse()` this removes/overrides any + // patches done by urlutil.URL in unsafe mode (ex: https://scanme.sh/%invalid) + // Note: this does not have any impact on actual url when sending request + httpReq, err := http.NewRequestWithContext(ctx, method, "https://example.com", nil) if err != nil { return nil, err } + urlx.Update() httpReq.URL = urlx.URL updateScheme(httpReq.URL) - // content-length and body should be assigned only // if request has body if bodyReader != nil { @@ -254,31 +259,22 @@ func NewRequest(method, url string, body interface{}) (*Request, error) { return &Request{httpReq, urlx, Metrics{}, nil}, nil } -// NewRequestWithContext creates a new wrapped request with context -func NewRequestWithContext(ctx context.Context, method, url string, body interface{}) (*Request, error) { - bodyReader, contentLength, err := getReusableBodyandContentLength(body) +// NewRequest creates a new wrapped request +func NewRequest(method, url string, body interface{}) (*Request, error) { + urlx, err := urlutil.Parse(url) if err != nil { return nil, err } + return NewRequestFromURL(method, urlx, body) +} +// NewRequest creates a new wrapped request with given context +func NewRequestWithContext(ctx context.Context, method, url string, body interface{}) (*Request, error) { urlx, err := urlutil.Parse(url) if err != nil { return nil, err } - httpReq, err := http.NewRequestWithContext(ctx, method, url, nil) - if err != nil { - return nil, err - } - httpReq.URL = urlx.URL - updateScheme(httpReq.URL) - // content-length and body should be assigned only - // if request has body - if bodyReader != nil { - httpReq.ContentLength = contentLength - httpReq.Body = bodyReader - } - - return &Request{httpReq, urlx, Metrics{}, nil}, nil + return NewRequestFromURLWithContext(ctx, method, urlx, body) } func updateScheme(u *url.URL) { From 54f3176b2916d4a5aeffa60a62d2fdfcd148fcdf Mon Sep 17 00:00:00 2001 From: Tarun Koyalwar Date: Wed, 8 Feb 2023 20:10:03 +0530 Subject: [PATCH 3/3] fix request host --- request.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/request.go b/request.go index 863d095..eec02c6 100644 --- a/request.go +++ b/request.go @@ -238,11 +238,12 @@ func NewRequestFromURLWithContext(ctx context.Context, method string, urlx *urlu return nil, err } - // we provide a dummy url at start and then replace url instance directly + // we provide a url without path to http.NewRequest at start and then replace url instance directly // because `http.NewRequest()` internally parses using `url.Parse()` this removes/overrides any // patches done by urlutil.URL in unsafe mode (ex: https://scanme.sh/%invalid) - // Note: this does not have any impact on actual url when sending request - httpReq, err := http.NewRequestWithContext(ctx, method, "https://example.com", nil) + // Note: this does not have any impact on actual path when sending request + // `http.NewRequestxxx` internally only uses `u.Host` and all other data is stored in `url.URL` instance + httpReq, err := http.NewRequestWithContext(ctx, method, "https://"+urlx.Host, nil) if err != nil { return nil, err }