Skip to content

Commit ba87210

Browse files
neildgopherbot
neild
authored and
gopherbot
committed
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 Fixes 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://go-review.googlesource.com/c/net/+/576155 Reviewed-by: Dmitri Shuralyov <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Than McIntosh <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent ebc8168 commit ba87210

File tree

2 files changed

+115
-0
lines changed

2 files changed

+115
-0
lines changed

‎http2/frame.go

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

‎http2/server_test.go

+84
Original file line numberDiff line numberDiff line change
@@ -4786,3 +4786,87 @@ Frames:
47864786
close(s)
47874787
}
47884788
}
4789+
4790+
func TestServerContinuationFlood(t *testing.T) {
4791+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
4792+
fmt.Println(r.Header)
4793+
}, func(ts *httptest.Server) {
4794+
ts.Config.MaxHeaderBytes = 4096
4795+
})
4796+
defer st.Close()
4797+
4798+
st.writePreface()
4799+
st.writeInitialSettings()
4800+
st.writeSettingsAck()
4801+
4802+
st.writeHeaders(HeadersFrameParam{
4803+
StreamID: 1,
4804+
BlockFragment: st.encodeHeader(),
4805+
EndStream: true,
4806+
})
4807+
for i := 0; i < 1000; i++ {
4808+
st.fr.WriteContinuation(1, false, st.encodeHeaderRaw(
4809+
fmt.Sprintf("x-%v", i), "1234567890",
4810+
))
4811+
}
4812+
st.fr.WriteContinuation(1, true, st.encodeHeaderRaw(
4813+
"x-last-header", "1",
4814+
))
4815+
4816+
var sawGoAway bool
4817+
for {
4818+
f, err := st.readFrame()
4819+
if err != nil {
4820+
break
4821+
}
4822+
switch f.(type) {
4823+
case *GoAwayFrame:
4824+
sawGoAway = true
4825+
case *HeadersFrame:
4826+
t.Fatalf("received HEADERS frame; want GOAWAY")
4827+
}
4828+
}
4829+
if !sawGoAway {
4830+
t.Errorf("connection closed with no GOAWAY frame; want one")
4831+
}
4832+
}
4833+
4834+
func TestServerContinuationAfterInvalidHeader(t *testing.T) {
4835+
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
4836+
fmt.Println(r.Header)
4837+
})
4838+
defer st.Close()
4839+
4840+
st.writePreface()
4841+
st.writeInitialSettings()
4842+
st.writeSettingsAck()
4843+
4844+
st.writeHeaders(HeadersFrameParam{
4845+
StreamID: 1,
4846+
BlockFragment: st.encodeHeader(),
4847+
EndStream: true,
4848+
})
4849+
st.fr.WriteContinuation(1, false, st.encodeHeaderRaw(
4850+
"x-invalid-header", "\x00",
4851+
))
4852+
st.fr.WriteContinuation(1, true, st.encodeHeaderRaw(
4853+
"x-valid-header", "1",
4854+
))
4855+
4856+
var sawGoAway bool
4857+
for {
4858+
f, err := st.readFrame()
4859+
if err != nil {
4860+
break
4861+
}
4862+
switch f.(type) {
4863+
case *GoAwayFrame:
4864+
sawGoAway = true
4865+
case *HeadersFrame:
4866+
t.Fatalf("received HEADERS frame; want GOAWAY")
4867+
}
4868+
}
4869+
if !sawGoAway {
4870+
t.Errorf("connection closed with no GOAWAY frame; want one")
4871+
}
4872+
}

0 commit comments

Comments
 (0)