[release-branch.go1.21] net/http, net/http/cookiejar: avoid subdomain matches on...

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 #65385
For #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://team-review.git.corp.google.com/c/golang/go-private/+/2173775
Reviewed-by: Carlos Amedee <amedee@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/569239
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 2cab53a585ab770d5cb502b7b510819f5293b37c..77a701b8061132a5748915c6f16c9da44cb25042 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -1014,6 +1014,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 0fe555af38f08a3b06101f11f2edb4a96d9e458d..fc1d79161248b693f225026055aaf0f7d8abc3f2 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -1725,6 +1725,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 273b54c84c7fde6f329640748a25c1ac84ee05c9..4b16266057b4e99d1079a24fb12508548c38889c 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) {