Skip to content

Commit 80f4377

Browse files
neildbradfitz
authored andcommitted
[release-branch.go1.22] 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 golang#65859 For golang#65065 Change-Id: I8f463f59f0e700c8a18733d2b264a8bcb3a19599 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2131938 Reviewed-by: Tatiana Bradley <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2174344 Reviewed-by: Carlos Amedee <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/569236 Reviewed-by: Carlos Amedee <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Michael Knyszek <[email protected]>
1 parent 4da1f86 commit 80f4377

File tree

4 files changed

+24
-0
lines changed

4 files changed

+24
-0
lines changed

src/net/http/client.go

+6
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,12 @@ func isDomainOrSubdomain(sub, parent string) bool {
10141014
if sub == parent {
10151015
return true
10161016
}
1017+
// If sub contains a :, it's probably an IPv6 address (and is definitely not a hostname).
1018+
// Don't check the suffix in this case, to avoid matching the contents of a IPv6 zone.
1019+
// For example, "::1%.www.example.com" is not a subdomain of "www.example.com".
1020+
if strings.ContainsAny(sub, ":%") {
1021+
return false
1022+
}
10171023
// If sub is "foo.example.com" and parent is "example.com",
10181024
// that means sub must end in "."+parent.
10191025
// Do it without allocating.

src/net/http/client_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -1711,6 +1711,7 @@ func TestShouldCopyHeaderOnRedirect(t *testing.T) {
17111711
{"authorization", "http://foo.com/", "https://foo.com/", true},
17121712
{"authorization", "http://foo.com:1234/", "http://foo.com:4321/", true},
17131713
{"www-authenticate", "http://foo.com/", "http://bar.com/", false},
1714+
{"authorization", "http://foo.com/", "http://[::1%25.foo.com]/", false},
17141715

17151716
// But subdomains should work:
17161717
{"www-authenticate", "http://foo.com/", "http://foo.com/", true},

src/net/http/cookiejar/jar.go

+7
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,13 @@ func jarKey(host string, psl PublicSuffixList) string {
362362

363363
// isIP reports whether host is an IP address.
364364
func isIP(host string) bool {
365+
if strings.ContainsAny(host, ":%") {
366+
// Probable IPv6 address.
367+
// Hostnames can't contain : or %, so this is definitely not a valid host.
368+
// Treating it as an IP is the more conservative option, and avoids the risk
369+
// of interpeting ::1%.www.example.com as a subtomain of www.example.com.
370+
return true
371+
}
365372
return net.ParseIP(host) != nil
366373
}
367374

src/net/http/cookiejar/jar_test.go

+10
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ var isIPTests = map[string]bool{
252252
"127.0.0.1": true,
253253
"1.2.3.4": true,
254254
"2001:4860:0:2001::68": true,
255+
"::1%zone": true,
255256
"example.com": false,
256257
"1.1.1.300": false,
257258
"www.foo.bar.net": false,
@@ -629,6 +630,15 @@ var basicsTests = [...]jarTest{
629630
{"http://www.host.test:1234/", "a=1"},
630631
},
631632
},
633+
{
634+
"IPv6 zone is not treated as a host.",
635+
"https://example.com/",
636+
[]string{"a=1"},
637+
"a=1",
638+
[]query{
639+
{"https://[::1%25.example.com]:80/", ""},
640+
},
641+
},
632642
}
633643

634644
func TestBasics(t *testing.T) {

0 commit comments

Comments
 (0)