proxy, http/httpproxy: do not mismatch IPv6 zone ids against hosts
When matching against a host "example.com",
don't match an IPv6 address like "[1000::1%25.example.com]:80".
Thanks to Juho Forsén of Mattermost for reporting this issue.
Fixes CVE-2025-22870
For #71984
Change-Id: I0c4fdf18765decc27e6ddf220ebe3a9bf4a6454d
Reviewed-on: https://go-review.googlesource.com/c/net/+/654697
Auto-Submit: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Roland Shoemaker <roland@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/http/httpproxy/proxy.go b/http/httpproxy/proxy.go
index 6404aaf..d89c257 100644
--- a/http/httpproxy/proxy.go
+++ b/http/httpproxy/proxy.go
@@ -14,6 +14,7 @@
"errors"
"fmt"
"net"
+ "net/netip"
"net/url"
"os"
"strings"
@@ -177,8 +178,10 @@
if host == "localhost" {
return false
}
- ip := net.ParseIP(host)
- if ip != nil {
+ nip, err := netip.ParseAddr(host)
+ var ip net.IP
+ if err == nil {
+ ip = net.IP(nip.AsSlice())
if ip.IsLoopback() {
return false
}
@@ -360,6 +363,9 @@
}
func (m domainMatch) match(host, port string, ip net.IP) bool {
+ if ip != nil {
+ return false
+ }
if strings.HasSuffix(host, m.host) || (m.matchHost && host == m.host[1:]) {
return m.port == "" || m.port == port
}
diff --git a/http/httpproxy/proxy_test.go b/http/httpproxy/proxy_test.go
index 790afda..a1dd2e8 100644
--- a/http/httpproxy/proxy_test.go
+++ b/http/httpproxy/proxy_test.go
@@ -211,6 +211,13 @@
},
req: "http://www.xn--fsq092h.com",
want: "<nil>",
+}, {
+ cfg: httpproxy.Config{
+ NoProxy: "example.com",
+ HTTPProxy: "proxy",
+ },
+ req: "http://[1000::%25.example.com]:123",
+ want: "http://proxy",
},
}
diff --git a/proxy/per_host.go b/proxy/per_host.go
index d7d4b8b..32bdf43 100644
--- a/proxy/per_host.go
+++ b/proxy/per_host.go
@@ -7,6 +7,7 @@
import (
"context"
"net"
+ "net/netip"
"strings"
)
@@ -57,7 +58,8 @@
}
func (p *PerHost) dialerForRequest(host string) Dialer {
- if ip := net.ParseIP(host); ip != nil {
+ if nip, err := netip.ParseAddr(host); err == nil {
+ ip := net.IP(nip.AsSlice())
for _, net := range p.bypassNetworks {
if net.Contains(ip) {
return p.bypass
@@ -108,8 +110,8 @@
}
continue
}
- if ip := net.ParseIP(host); ip != nil {
- p.AddIP(ip)
+ if nip, err := netip.ParseAddr(host); err == nil {
+ p.AddIP(net.IP(nip.AsSlice()))
continue
}
if strings.HasPrefix(host, "*.") {
diff --git a/proxy/per_host_test.go b/proxy/per_host_test.go
index 0447eb4..b7bcec8 100644
--- a/proxy/per_host_test.go
+++ b/proxy/per_host_test.go
@@ -7,8 +7,9 @@
import (
"context"
"errors"
+ "fmt"
"net"
- "reflect"
+ "slices"
"testing"
)
@@ -22,55 +23,118 @@
}
func TestPerHost(t *testing.T) {
- expectedDef := []string{
- "example.com:123",
- "1.2.3.4:123",
- "[1001::]:123",
+ for _, test := range []struct {
+ config string // passed to PerHost.AddFromString
+ nomatch []string // addrs using the default dialer
+ match []string // addrs using the bypass dialer
+ }{{
+ config: "localhost,*.zone,127.0.0.1,10.0.0.1/8,1000::/16",
+ nomatch: []string{
+ "example.com:123",
+ "1.2.3.4:123",
+ "[1001::]:123",
+ },
+ match: []string{
+ "localhost:123",
+ "zone:123",
+ "foo.zone:123",
+ "127.0.0.1:123",
+ "10.1.2.3:123",
+ "[1000::]:123",
+ "[1000::%25.example.com]:123",
+ },
+ }, {
+ config: "localhost",
+ nomatch: []string{
+ "127.0.0.1:80",
+ },
+ match: []string{
+ "localhost:80",
+ },
+ }, {
+ config: "*.zone",
+ nomatch: []string{
+ "foo.com:80",
+ },
+ match: []string{
+ "foo.zone:80",
+ "foo.bar.zone:80",
+ },
+ }, {
+ config: "1.2.3.4",
+ nomatch: []string{
+ "127.0.0.1:80",
+ "11.2.3.4:80",
+ },
+ match: []string{
+ "1.2.3.4:80",
+ },
+ }, {
+ config: "10.0.0.0/24",
+ nomatch: []string{
+ "10.0.1.1:80",
+ },
+ match: []string{
+ "10.0.0.1:80",
+ "10.0.0.255:80",
+ },
+ }, {
+ config: "fe80::/10",
+ nomatch: []string{
+ "[fec0::1]:80",
+ "[fec0::1%en0]:80",
+ },
+ match: []string{
+ "[fe80::1]:80",
+ "[fe80::1%en0]:80",
+ },
+ }, {
+ // We don't allow zone IDs in network prefixes,
+ // so this config matches nothing.
+ config: "fe80::%en0/10",
+ nomatch: []string{
+ "[fec0::1]:80",
+ "[fec0::1%en0]:80",
+ "[fe80::1]:80",
+ "[fe80::1%en0]:80",
+ "[fe80::1%en1]:80",
+ },
+ }} {
+ for _, addr := range test.match {
+ testPerHost(t, test.config, addr, true)
+ }
+ for _, addr := range test.nomatch {
+ testPerHost(t, test.config, addr, false)
+ }
}
- expectedBypass := []string{
- "localhost:123",
- "zone:123",
- "foo.zone:123",
- "127.0.0.1:123",
- "10.1.2.3:123",
- "[1000::]:123",
+}
+
+func testPerHost(t *testing.T, config, addr string, wantMatch bool) {
+ name := fmt.Sprintf("config %q, dial %q", config, addr)
+
+ var def, bypass recordingProxy
+ perHost := NewPerHost(&def, &bypass)
+ perHost.AddFromString(config)
+ perHost.Dial("tcp", addr)
+
+ // Dial and DialContext should have the same results.
+ var defc, bypassc recordingProxy
+ perHostc := NewPerHost(&defc, &bypassc)
+ perHostc.AddFromString(config)
+ perHostc.DialContext(context.Background(), "tcp", addr)
+ if !slices.Equal(def.addrs, defc.addrs) {
+ t.Errorf("%v: Dial default=%v, bypass=%v; DialContext default=%v, bypass=%v", name, def.addrs, bypass.addrs, defc.addrs, bypass.addrs)
+ return
}
- t.Run("Dial", func(t *testing.T) {
- var def, bypass recordingProxy
- perHost := NewPerHost(&def, &bypass)
- perHost.AddFromString("localhost,*.zone,127.0.0.1,10.0.0.1/8,1000::/16")
- for _, addr := range expectedDef {
- perHost.Dial("tcp", addr)
- }
- for _, addr := range expectedBypass {
- perHost.Dial("tcp", addr)
- }
+ if got, want := slices.Concat(def.addrs, bypass.addrs), []string{addr}; !slices.Equal(got, want) {
+ t.Errorf("%v: dialed %q, want %q", name, got, want)
+ return
+ }
- if !reflect.DeepEqual(expectedDef, def.addrs) {
- t.Errorf("Hosts which went to the default proxy didn't match. Got %v, want %v", def.addrs, expectedDef)
- }
- if !reflect.DeepEqual(expectedBypass, bypass.addrs) {
- t.Errorf("Hosts which went to the bypass proxy didn't match. Got %v, want %v", bypass.addrs, expectedBypass)
- }
- })
-
- t.Run("DialContext", func(t *testing.T) {
- var def, bypass recordingProxy
- perHost := NewPerHost(&def, &bypass)
- perHost.AddFromString("localhost,*.zone,127.0.0.1,10.0.0.1/8,1000::/16")
- for _, addr := range expectedDef {
- perHost.DialContext(context.Background(), "tcp", addr)
- }
- for _, addr := range expectedBypass {
- perHost.DialContext(context.Background(), "tcp", addr)
- }
-
- if !reflect.DeepEqual(expectedDef, def.addrs) {
- t.Errorf("Hosts which went to the default proxy didn't match. Got %v, want %v", def.addrs, expectedDef)
- }
- if !reflect.DeepEqual(expectedBypass, bypass.addrs) {
- t.Errorf("Hosts which went to the bypass proxy didn't match. Got %v, want %v", bypass.addrs, expectedBypass)
- }
- })
+ gotMatch := len(bypass.addrs) > 0
+ if gotMatch != wantMatch {
+ t.Errorf("%v: matched=%v, want %v", name, gotMatch, wantMatch)
+ return
+ }
}
