Skip to content

Commit

Permalink
proxy, http/httpproxy: do not mismatch IPv6 zone ids against hosts
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
neild authored and gopherbot committed Mar 4, 2025
1 parent fe7f039 commit cde1dda
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 52 deletions.
10 changes: 8 additions & 2 deletions http/httpproxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"errors"
"fmt"
"net"
"net/netip"
"net/url"
"os"
"strings"
Expand Down Expand Up @@ -177,8 +178,10 @@ func (cfg *config) useProxy(addr string) bool {
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
}
Expand Down Expand Up @@ -360,6 +363,9 @@ type domainMatch struct {
}

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
}
Expand Down
7 changes: 7 additions & 0 deletions http/httpproxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,13 @@ var proxyForURLTests = []proxyForURLTest{{
},
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",
},
}

Expand Down
8 changes: 5 additions & 3 deletions proxy/per_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package proxy
import (
"context"
"net"
"net/netip"
"strings"
)

Expand Down Expand Up @@ -57,7 +58,8 @@ func (p *PerHost) DialContext(ctx context.Context, network, addr string) (c net.
}

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
Expand Down Expand Up @@ -108,8 +110,8 @@ func (p *PerHost) AddFromString(s string) {
}
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, "*.") {
Expand Down
158 changes: 111 additions & 47 deletions proxy/per_host_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ package proxy
import (
"context"
"errors"
"fmt"
"net"
"reflect"
"slices"
"testing"
)

Expand All @@ -22,55 +23,118 @@ func (r *recordingProxy) Dial(network, addr string) (net.Conn, error) {
}

func TestPerHost(t *testing.T) {
expectedDef := []string{
"example.com:123",
"1.2.3.4:123",
"[1001::]:123",
}
expectedBypass := []string{
"localhost:123",
"zone:123",
"foo.zone:123",
"127.0.0.1:123",
"10.1.2.3:123",
"[1000::]:123",
}

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 _, 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 expectedBypass {
perHost.Dial("tcp", addr)
for _, addr := range test.nomatch {
testPerHost(t, test.config, addr, false)
}
}
}

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)
}
})
func testPerHost(t *testing.T, config, addr string, wantMatch bool) {
name := fmt.Sprintf("config %q, dial %q", config, addr)

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)
}
var def, bypass recordingProxy
perHost := NewPerHost(&def, &bypass)
perHost.AddFromString(config)
perHost.Dial("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)
}
})
// 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
}

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
}

gotMatch := len(bypass.addrs) > 0
if gotMatch != wantMatch {
t.Errorf("%v: matched=%v, want %v", name, gotMatch, wantMatch)
return
}
}

0 comments on commit cde1dda

Please sign in to comment.