net/http, net/http/cookiejar: avoid subdomain matches on IPv6 zones

When deciding whether to forward cookies or sensitive headers
across a redirect, do not attempt to interpret an IPv6 address
as a domain name.

Avoids a case where a maliciously-crafted redirect to an
IPv6 address with a scoped addressing zone could be
misinterpreted as a within-domain redirect. For example,
we could interpret "::1%.www.example.com" as a subdomain
of "www.example.com".

Thanks to Juho Nurminen of Mattermost for reporting this issue.

Fixes CVE-2023-45289
Fixes #65065

Change-Id: I8f463f59f0e700c8a18733d2b264a8bcb3a19599
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2131938
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569340
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 99ed7dc927faf609431ce2d23062ce9a28bdd7f8..0f29dbb2c5e4d6b68b4d810ca7cbad61885a4699 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -1008,6 +1008,12 @@ func isDomainOrSubdomain(sub, parent string) bool {
        if sub == parent {
                return true
        }
+       // If sub contains a :, it's probably an IPv6 address (and is definitely not a hostname).
+       // Don't check the suffix in this case, to avoid matching the contents of a IPv6 zone.
+       // For example, "::1%.www.example.com" is not a subdomain of "www.example.com".
+       if strings.ContainsAny(sub, ":%") {
+               return false
+       }
        // If sub is "foo.example.com" and parent is "example.com",
        // that means sub must end in "."+parent.
        // Do it without allocating.

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index 80a6664d3b55880cc623c44529628a1e7e2e4d1a..569b58ca6225c9de7d13ac3d0d1bfb0664828a93 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -1717,6 +1717,7 @@ func TestShouldCopyHeaderOnRedirect(t *testing.T) {
                {"authorization", "http://foo.com/", "https://foo.com/", true},
                {"authorization", "http://foo.com:1234/", "http://foo.com:4321/", true},
                {"www-authenticate", "http://foo.com/", "http://bar.com/", false},
+               {"authorization", "http://foo.com/", "http://[::1%25.foo.com]/", false},
 
                // But subdomains should work:
                {"www-authenticate", "http://foo.com/", "http://foo.com/", true},

diff --git a/src/net/http/cookiejar/jar.go b/src/net/http/cookiejar/jar.go
index 59cde82cb380e30236698079763b37838c1ae96f..e7f5ddd4d0096b9d036d6b7f4258d5695d9d9761 100644 (file)
--- a/src/net/http/cookiejar/jar.go
+++ b/src/net/http/cookiejar/jar.go
@@ -362,6 +362,13 @@ func jarKey(host string, psl PublicSuffixList) string {
 
 // isIP reports whether host is an IP address.
 func isIP(host string) bool {
+       if strings.ContainsAny(host, ":%") {
+               // Probable IPv6 address.
+               // Hostnames can't contain : or %, so this is definitely not a valid host.
+               // Treating it as an IP is the more conservative option, and avoids the risk
+               // of interpeting ::1%.www.example.com as a subtomain of www.example.com.
+               return true
+       }
        return net.ParseIP(host) != nil
 }
 

diff --git a/src/net/http/cookiejar/jar_test.go b/src/net/http/cookiejar/jar_test.go
index 56d0695a660c07415a6f7cdd68993428719e06ae..251f7c16171c3acc40be12a962eb7a48529699f8 100644 (file)
--- a/src/net/http/cookiejar/jar_test.go
+++ b/src/net/http/cookiejar/jar_test.go
@@ -252,6 +252,7 @@ var isIPTests = map[string]bool{
        "127.0.0.1":            true,
        "1.2.3.4":              true,
        "2001:4860:0:2001::68": true,
+       "::1%zone":             true,
        "example.com":          false,
        "1.1.1.300":            false,
        "www.foo.bar.net":      false,
@@ -629,6 +630,15 @@ var basicsTests = [...]jarTest{
                        {"http://www.host.test:1234/", "a=1"},
                },
        },
+       {
+               "IPv6 zone is not treated as a host.",
+               "https://example.com/",
+               []string{"a=1"},
+               "a=1",
+               []query{
+                       {"https://[::1%25.example.com]:80/", ""},
+               },
+       },
 }
 
 func TestBasics(t *testing.T) {