From 5741608de26f06488e771fd7b3b35ca2ca4c93a6 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 8 Jan 2026 10:25:44 +0100 Subject: net: don't ignore errors in TestUnixUnlink TestUnixUnlink calls some functions and methods that can fail, but it ignores the returned errors. This test is flaky on Windows, and those errors should be checked to help diagnose the problem. Updates #75282 Updates #76582 Updates #77038 Change-Id: Ia868762a4c0b94a7255d57add63777568caa6cd2 Reviewed-on: https://go-review.googlesource.com/c/go/+/734720 Reviewed-by: Florian Lehner LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Reviewed-by: Dmitri Shuralyov --- src/net/unixsock_test.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'src/net') diff --git a/src/net/unixsock_test.go b/src/net/unixsock_test.go index f6c5679f42..3e4a14b255 100644 --- a/src/net/unixsock_test.go +++ b/src/net/unixsock_test.go @@ -375,6 +375,17 @@ func TestUnixUnlink(t *testing.T) { } return l.(*UnixListener) } + fileListener := func(t *testing.T, l *UnixListener) (*os.File, Listener) { + f, err := l.File() + if err != nil { + t.Fatal(err) + } + ln, err := FileListener(f) + if err != nil { + t.Fatal(err) + } + return f, ln + } checkExists := func(t *testing.T, desc string) { if _, err := os.Stat(name); err != nil { t.Fatalf("unix socket does not exist %s: %v", desc, err) @@ -397,8 +408,7 @@ func TestUnixUnlink(t *testing.T) { // FileListener should not. t.Run("FileListener", func(t *testing.T) { l := listen(t) - f, _ := l.File() - l1, _ := FileListener(f) + f, l1 := fileListener(t, l) checkExists(t, "after FileListener") f.Close() checkExists(t, "after File close") @@ -444,8 +454,7 @@ func TestUnixUnlink(t *testing.T) { t.Run("FileListener/SetUnlinkOnClose(true)", func(t *testing.T) { l := listen(t) - f, _ := l.File() - l1, _ := FileListener(f) + f, l1 := fileListener(t, l) checkExists(t, "after FileListener") l1.(*UnixListener).SetUnlinkOnClose(true) f.Close() @@ -457,8 +466,7 @@ func TestUnixUnlink(t *testing.T) { t.Run("FileListener/SetUnlinkOnClose(false)", func(t *testing.T) { l := listen(t) - f, _ := l.File() - l1, _ := FileListener(f) + f, l1 := fileListener(t, l) checkExists(t, "after FileListener") l1.(*UnixListener).SetUnlinkOnClose(false) f.Close() -- cgit v1.3 From 30d0b4026410da3486ba841bb7f13c1d9074e91d Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 8 Jan 2026 15:44:51 +0100 Subject: net: don't ignore getsockname errors in newFileFD newFileFD is called when creating a net FD from an existing socket handle. That socket might not be bound yet, in which case getsockname returns a useful error that is currently ignored and replaced with a potentially misleading EPROTONOSUPPORT error later on. Updates #73696 Updates #74976 Updates #75282 Updates #75279 Updates #76537 Updates #76582 Updates #77038 Change-Id: I2a8b30ffbb037035669f65a95a923edc8b288145 Reviewed-on: https://go-review.googlesource.com/c/go/+/734820 LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-by: Damien Neil --- src/net/file_posix.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src/net') diff --git a/src/net/file_posix.go b/src/net/file_posix.go index 132d03e9e3..b28ff5845d 100644 --- a/src/net/file_posix.go +++ b/src/net/file_posix.go @@ -23,7 +23,11 @@ func newFileFD(f *os.File) (*netFD, error) { poll.CloseFunc(s) return nil, os.NewSyscallError("getsockopt", err) } - lsa, _ := syscall.Getsockname(s) + lsa, err := syscall.Getsockname(s) + if err != nil { + poll.CloseFunc(s) + return nil, os.NewSyscallError("getsockname", err) + } rsa, _ := syscall.Getpeername(s) switch lsa.(type) { case *syscall.SockaddrInet4: -- cgit v1.3 From cbe153806e67a16e362a1cdbbf1741d4ce82e98a Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 8 Jan 2026 15:50:15 +0100 Subject: net: fix socket duplication error handling on Windows Calls to dupSocket may fail, but the error is not properly handled because the surrounding code incorrectly checks for nil error instead of non-nil error. I'm not aware of any code paths that would trigger this error, and I haven't been able to create a test case that does so, but this change fixes the error handling to correctly propagate any errors from dupSocket. Change-Id: I5ffd3cbe8ed58a83634f3b97c0878a7c73e0505e Reviewed-on: https://go-review.googlesource.com/c/go/+/734821 Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil --- src/net/fd_windows.go | 2 +- src/net/file_windows.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/net') diff --git a/src/net/fd_windows.go b/src/net/fd_windows.go index 52985be8e6..c28ce045c4 100644 --- a/src/net/fd_windows.go +++ b/src/net/fd_windows.go @@ -248,7 +248,7 @@ func (fd *netFD) dup() (*os.File, error) { err := fd.pfd.RawControl(func(fd uintptr) { h, syserr = dupSocket(syscall.Handle(fd)) }) - if err != nil { + if err == nil { err = syserr } if err != nil { diff --git a/src/net/file_windows.go b/src/net/file_windows.go index b4eb00e564..6a6305035a 100644 --- a/src/net/file_windows.go +++ b/src/net/file_windows.go @@ -39,7 +39,7 @@ func dupFileSocket(f *os.File) (syscall.Handle, error) { err = sc.Control(func(fd uintptr) { h, syserr = dupSocket(syscall.Handle(fd)) }) - if err != nil { + if err == nil { err = syserr } if err != nil { -- cgit v1.3 From 2dcaaa751295597e1f603b7488c4624db6a84d2b Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Mon, 3 Nov 2025 14:28:47 -0800 Subject: net/url: add urlmaxqueryparams GODEBUG to limit the number of query parameters net/url does not currently limit the number of query parameters parsed by url.ParseQuery or URL.Query. When parsing a application/x-www-form-urlencoded form, net/http.Request.ParseForm will parse up to 10 MB of query parameters. An input consisting of a large number of small, unique parameters can cause excessive memory consumption. We now limit the number of query parameters parsed to 10000 by default. The limit can be adjusted by setting GODEBUG=urlmaxqueryparams=. Setting urlmaxqueryparams to 0 disables the limit. Thanks to jub0bs for reporting this issue. Fixes #77101 Fixes CVE-2025-61726 Change-Id: Iee3374c7ee2d8586dbf158536d3ade424203ff66 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3020 Reviewed-by: Nicholas Husin Reviewed-by: Neal Patel Reviewed-on: https://go-review.googlesource.com/c/go/+/736712 Auto-Submit: Michael Pratt Reviewed-by: Junyang Shao LUCI-TryBot-Result: Go LUCI --- doc/godebug.md | 7 ++++++ src/internal/godebugs/table.go | 1 + src/net/url/url.go | 23 ++++++++++++++++++++ src/net/url/url_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ src/runtime/metrics/doc.go | 5 +++++ 5 files changed, 84 insertions(+) (limited to 'src/net') diff --git a/doc/godebug.md b/doc/godebug.md index 28a2dc506e..184e161c40 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -163,6 +163,13 @@ will fail early. The default value is `httpcookiemaxnum=3000`. Setting number of cookies. To avoid denial of service attacks, this setting and default was backported to Go 1.25.2 and Go 1.24.8. +Go 1.26 added a new `urlmaxqueryparams` setting that controls the maximum number +of query parameters that net/url will accept when parsing a URL-encoded query string. +If the number of parameters exceeds the number set in `urlmaxqueryparams`, +parsing will fail early. The default value is `urlmaxqueryparams=10000`. +Setting `urlmaxqueryparams=0`bles the limit. To avoid denial of service attacks, +this setting and default was backported to Go 1.25.4 and Go 1.24.10. + Go 1.26 added a new `urlstrictcolons` setting that controls whether `net/url.Parse` allows malformed hostnames containing colons outside of a bracketed IPv6 address. The default `urlstrictcolons=1` rejects URLs such as `http://localhost:1:2` or `http://::1/`. diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 8f6d8bbdda..87b499385a 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -69,6 +69,7 @@ var All = []Info{ {Name: "tlssha1", Package: "crypto/tls", Changed: 25, Old: "1"}, {Name: "tlsunsafeekm", Package: "crypto/tls", Changed: 22, Old: "1"}, {Name: "updatemaxprocs", Package: "runtime", Changed: 25, Old: "0"}, + {Name: "urlmaxqueryparams", Package: "net/url", Changed: 24, Old: "0"}, {Name: "urlstrictcolons", Package: "net/url", Changed: 26, Old: "0"}, {Name: "winreadlinkvolume", Package: "os", Changed: 23, Old: "0"}, {Name: "winsymlink", Package: "os", Changed: 23, Old: "0"}, diff --git a/src/net/url/url.go b/src/net/url/url.go index 3acd202c24..202957a3a2 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -929,7 +929,30 @@ func ParseQuery(query string) (Values, error) { return m, err } +var urlmaxqueryparams = godebug.New("urlmaxqueryparams") + +const defaultMaxParams = 10000 + +func urlParamsWithinMax(params int) bool { + withinDefaultMax := params <= defaultMaxParams + if urlmaxqueryparams.Value() == "" { + return withinDefaultMax + } + customMax, err := strconv.Atoi(urlmaxqueryparams.Value()) + if err != nil { + return withinDefaultMax + } + withinCustomMax := customMax == 0 || params < customMax + if withinDefaultMax != withinCustomMax { + urlmaxqueryparams.IncNonDefault() + } + return withinCustomMax +} + func parseQuery(m Values, query string) (err error) { + if !urlParamsWithinMax(strings.Count(query, "&") + 1) { + return errors.New("number of URL query parameters exceeded limit") + } for query != "" { var key string key, query, _ = strings.Cut(query, "&") diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index bb48bb6bee..d099353eb2 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -1521,6 +1521,54 @@ func TestParseQuery(t *testing.T) { } } +func TestParseQueryLimits(t *testing.T) { + for _, test := range []struct { + params int + godebug string + wantErr bool + }{{ + params: 10, + wantErr: false, + }, { + params: defaultMaxParams, + wantErr: false, + }, { + params: defaultMaxParams + 1, + wantErr: true, + }, { + params: 10, + godebug: "urlmaxqueryparams=9", + wantErr: true, + }, { + params: defaultMaxParams + 1, + godebug: "urlmaxqueryparams=0", + wantErr: false, + }} { + t.Setenv("GODEBUG", test.godebug) + want := Values{} + var b strings.Builder + for i := range test.params { + if i > 0 { + b.WriteString("&") + } + p := fmt.Sprintf("p%v", i) + b.WriteString(p) + want[p] = []string{""} + } + query := b.String() + got, err := ParseQuery(query) + if gotErr, wantErr := err != nil, test.wantErr; gotErr != wantErr { + t.Errorf("GODEBUG=%v ParseQuery(%v params) = %v, want error: %v", test.godebug, test.params, err, wantErr) + } + if err != nil { + continue + } + if got, want := len(got), test.params; got != want { + t.Errorf("GODEBUG=%v ParseQuery(%v params): got %v params, want %v", test.godebug, test.params, got, want) + } + } +} + type RequestURITest struct { url *URL out string diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index ca032f51b1..6b774c36f3 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -404,6 +404,11 @@ Below is the full list of supported metrics, ordered lexicographically. The number of non-default behaviors executed by the runtime package due to a non-default GODEBUG=updatemaxprocs=... setting. + /godebug/non-default-behavior/urlmaxqueryparams:events + The number of non-default behaviors executed by the net/url + package due to a non-default GODEBUG=urlmaxqueryparams=... + setting. + /godebug/non-default-behavior/urlstrictcolons:events The number of non-default behaviors executed by the net/url package due to a non-default GODEBUG=urlstrictcolons=... -- cgit v1.3