From 04e3ece5b380ecad9da3551c449f1b8a9aa76d3d Mon Sep 17 00:00:00 2001 From: roc Date: Wed, 8 May 2024 14:16:57 +0800 Subject: [PATCH] prevent successful requests from invalid host --- http.go | 40 ++++------------------------------------ transport.go | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/http.go b/http.go index ff99699f..61a63676 100644 --- a/http.go +++ b/http.go @@ -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 @@ -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 { diff --git a/transport.go b/transport.go index 620e2923..a1f847b0 100644 --- a/transport.go +++ b/transport.go @@ -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