diff options
| author | Nicholas S. Husin <nsh@golang.org> | 2026-03-31 15:02:11 -0400 |
|---|---|---|
| committer | Nicholas Husin <nsh@golang.org> | 2026-03-31 15:18:14 -0700 |
| commit | e3a10fe3749eea3d7d5f284c3a1e57479131176f (patch) | |
| tree | 60a419181715c4088366bdc179a2b59ccf8db398 /src/net/http | |
| parent | 836b0984ee0b930c02ca62f9e8801f1016c9b253 (diff) | |
| download | go-e3a10fe3749eea3d7d5f284c3a1e57479131176f.tar.xz | |
net/http/internal/http2: prevent hanging Transport due to bad SETTINGS frame
When processing SETTINGS frame, Transport currently only checks if the
frame is valid for SETTINGS_ENABLE_CONNECT_PROTOCOL. As a result, a
SETTINGS_MAX_FRAME_SIZE with the invalid value of 0 is erroneously
accepted. This will then result in Transport being stuck in an infinite
loop writing CONTINUATION frames.
This CL fixes the issue by ensuring that SETTINGS frame are always
validated, regardless of the SETTINGS parameter.
Thanks to Marwan Atia (marwansamir688@gmail.com) for reporting this
issue.
Fixes #78476
Fixes CVE-2026-33814
Change-Id: I8b6219431e87454d34bca738fbcb59b66a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/761581
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Diffstat (limited to 'src/net/http')
| -rw-r--r-- | src/net/http/internal/http2/transport.go | 6 | ||||
| -rw-r--r-- | src/net/http/internal/http2/transport_test.go | 13 |
2 files changed, 16 insertions, 3 deletions
diff --git a/src/net/http/internal/http2/transport.go b/src/net/http/internal/http2/transport.go index 36423b22df..2b04bed50a 100644 --- a/src/net/http/internal/http2/transport.go +++ b/src/net/http/internal/http2/transport.go @@ -2836,6 +2836,9 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { var seenMaxConcurrentStreams bool err := f.ForeachSetting(func(s Setting) error { + if err := s.Valid(); err != nil { + return err + } switch s.ID { case SettingMaxFrameSize: cc.maxFrameSize = s.Val @@ -2867,9 +2870,6 @@ func (rl *clientConnReadLoop) processSettingsNoWrite(f *SettingsFrame) error { cc.henc.SetMaxDynamicTableSize(s.Val) cc.peerMaxHeaderTableSize = s.Val case SettingEnableConnectProtocol: - if err := s.Valid(); err != nil { - return err - } // If the peer wants to send us SETTINGS_ENABLE_CONNECT_PROTOCOL, // we require that it do so in the first SETTINGS frame. // diff --git a/src/net/http/internal/http2/transport_test.go b/src/net/http/internal/http2/transport_test.go index 2e3dad3310..955c2208ad 100644 --- a/src/net/http/internal/http2/transport_test.go +++ b/src/net/http/internal/http2/transport_test.go @@ -5428,6 +5428,19 @@ func testTransportTLSNextProtoConnImmediateFailureUnused(t testing.TB) { tc2.wantFrameType(FrameHeaders) } +func TestTransportDoNotHangOnZeroMaxFrameSize(t *testing.T) { + synctestTest(t, testTransportDoNotHangOnZeroMaxFrameSize) +} +func testTransportDoNotHangOnZeroMaxFrameSize(t testing.TB) { + tc := newTestClientConn(t) + tc.writeSettings(Setting{ID: SettingMaxFrameSize, Val: 0}) + tc.wantFrameType(FrameSettings) + + req, _ := http.NewRequest("POST", "https://dummy.tld/", strings.NewReader("body")) + tc.roundTrip(req) + // Previously, https://go.dev/issue/78476 caused an infinite hang here. +} + func TestExtendedConnectClientWithServerSupport(t *testing.T) { t.Skip("https://go.dev/issue/53208 -- net/http needs to support the :protocol header") SetDisableExtendedConnectProtocol(t, false) |