Skip to content

Commit 947e999

Browse files
neildgopherbot
authored andcommitted
[internal-branch.go1.21-vendor] http2: close connections when receiving too many headers
Maintaining HPACK state requires that we parse and process all HEADERS and CONTINUATION frames on a connection. When a request's headers exceed MaxHeaderBytes, we don't allocate memory to store the excess headers but we do parse them. This permits an attacker to cause an HTTP/2 endpoint to read arbitrary amounts of data, all associated with a request which is going to be rejected. Set a limit on the amount of excess header frames we will process before closing a connection. Thanks to Bartek Nowotarski for reporting this issue. Fixes CVE-2023-45288 For golang/go#65051 Change-Id: I15df097268df13bb5a9e9d3a5c04a8a141d850f6 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2130527 Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: Tatiana Bradley <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2197243 Run-TryBot: Damien Neil <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/net/+/576057 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]>
1 parent 1a2eef3 commit 947e999

File tree

2 files changed

+115
-0
lines changed

2 files changed

+115
-0
lines changed

http2/frame.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,7 @@ func (fr *Framer) readMetaFrame(hf *HeadersFrame) (*MetaHeadersFrame, error) {
15651565
if size > remainSize {
15661566
hdec.SetEmitEnabled(false)
15671567
mh.Truncated = true
1568+
remainSize = 0
15681569
return
15691570
}
15701571
remainSize -= size
@@ -1577,6 +1578,36 @@ func (fr *Framer) readMetaFrame(hf *HeadersFrame) (*MetaHeadersFrame, error) {
15771578
var hc headersOrContinuation = hf
15781579
for {
15791580
frag := hc.HeaderBlockFragment()
1581+
1582+
// Avoid parsing large amounts of headers that we will then discard.
1583+
// If the sender exceeds the max header list size by too much,
1584+
// skip parsing the fragment and close the connection.
1585+
//
1586+
// "Too much" is either any CONTINUATION frame after we've already
1587+
// exceeded the max header list size (in which case remainSize is 0),
1588+
// or a frame whose encoded size is more than twice the remaining
1589+
// header list bytes we're willing to accept.
1590+
if int64(len(frag)) > int64(2*remainSize) {
1591+
if VerboseLogs {
1592+
log.Printf("http2: header list too large")
1593+
}
1594+
// It would be nice to send a RST_STREAM before sending the GOAWAY,
1595+
// but the struture of the server's frame writer makes this difficult.
1596+
return nil, ConnectionError(ErrCodeProtocol)
1597+
}
1598+
1599+
// Also close the connection after any CONTINUATION frame following an
1600+
// invalid header, since we stop tracking the size of the headers after
1601+
// an invalid one.
1602+
if invalid != nil {
1603+
if VerboseLogs {
1604+
log.Printf("http2: invalid header: %v", invalid)
1605+
}
1606+
// It would be nice to send a RST_STREAM before sending the GOAWAY,
1607+
// but the struture of the server's frame writer makes this difficult.
1608+
return nil, ConnectionError(ErrCodeProtocol)
1609+
}
1610+
15801611
if _, err := hdec.Write(frag); err != nil {
15811612
return nil, ConnectionError(ErrCodeCompression)
15821613
}

http2/server_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4869,3 +4869,87 @@ Frames:
48694869
close(s)
48704870
}
48714871
}
4872+
4873+
func TestServerContinuationFlood(t *testing.T) {
4874+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
4875+
fmt.Println(r.Header)
4876+
}, func(ts *httptest.Server) {
4877+
ts.Config.MaxHeaderBytes = 4096
4878+
})
4879+
defer st.Close()
4880+
4881+
st.writePreface()
4882+
st.writeInitialSettings()
4883+
st.writeSettingsAck()
4884+
4885+
st.writeHeaders(HeadersFrameParam{
4886+
StreamID: 1,
4887+
BlockFragment: st.encodeHeader(),
4888+
EndStream: true,
4889+
})
4890+
for i := 0; i < 1000; i++ {
4891+
st.fr.WriteContinuation(1, false, st.encodeHeaderRaw(
4892+
fmt.Sprintf("x-%v", i), "1234567890",
4893+
))
4894+
}
4895+
st.fr.WriteContinuation(1, true, st.encodeHeaderRaw(
4896+
"x-last-header", "1",
4897+
))
4898+
4899+
var sawGoAway bool
4900+
for {
4901+
f, err := st.readFrame()
4902+
if err != nil {
4903+
break
4904+
}
4905+
switch f.(type) {
4906+
case *GoAwayFrame:
4907+
sawGoAway = true
4908+
case *HeadersFrame:
4909+
t.Fatalf("received HEADERS frame; want GOAWAY")
4910+
}
4911+
}
4912+
if !sawGoAway {
4913+
t.Errorf("connection closed with no GOAWAY frame; want one")
4914+
}
4915+
}
4916+
4917+
func TestServerContinuationAfterInvalidHeader(t *testing.T) {
4918+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
4919+
fmt.Println(r.Header)
4920+
})
4921+
defer st.Close()
4922+
4923+
st.writePreface()
4924+
st.writeInitialSettings()
4925+
st.writeSettingsAck()
4926+
4927+
st.writeHeaders(HeadersFrameParam{
4928+
StreamID: 1,
4929+
BlockFragment: st.encodeHeader(),
4930+
EndStream: true,
4931+
})
4932+
st.fr.WriteContinuation(1, false, st.encodeHeaderRaw(
4933+
"x-invalid-header", "\x00",
4934+
))
4935+
st.fr.WriteContinuation(1, true, st.encodeHeaderRaw(
4936+
"x-valid-header", "1",
4937+
))
4938+
4939+
var sawGoAway bool
4940+
for {
4941+
f, err := st.readFrame()
4942+
if err != nil {
4943+
break
4944+
}
4945+
switch f.(type) {
4946+
case *GoAwayFrame:
4947+
sawGoAway = true
4948+
case *HeadersFrame:
4949+
t.Fatalf("received HEADERS frame; want GOAWAY")
4950+
}
4951+
}
4952+
if !sawGoAway {
4953+
t.Errorf("connection closed with no GOAWAY frame; want one")
4954+
}
4955+
}

0 commit comments

Comments
 (0)