Merge pull request #2085 from lucas-clemente/harden-http3

check the size of HTTP3 HEADERS frames before allocating

http3/client.go

@@ -17,6 +17,7 @@ import (
 )
 
 const defaultUserAgent = "quic-go HTTP/3"
+const defaultMaxResponseHeaderBytes = 10 * 1 << 20 // 10 MB
 
 var defaultQuicConfig = &quic.Config{KeepAlive: true}
 
@@ -24,6 +25,7 @@ var dialAddr = quic.DialAddr
 
 type roundTripperOpts struct {
 	DisableCompression bool
+	MaxHeaderBytes     int64
 }
 
 // client is a HTTP3 client doing requests
@@ -121,6 +123,13 @@ func (c *client) Close() error {
 	return c.session.Close()
 }
 
+func (c *client) maxHeaderBytes() uint64 {
+	if c.opts.MaxHeaderBytes <= 0 {
+		return defaultMaxResponseHeaderBytes
+	}
+	return uint64(c.opts.MaxHeaderBytes)
+}
+
 // Roundtrip executes a request and returns a response
 // TODO: handle request cancelations
 func (c *client) RoundTrip(req *http.Request) (*http.Response, error) {
@@ -160,7 +169,9 @@ func (c *client) RoundTrip(req *http.Request) (*http.Response, error) {
 	if !ok {
 		return nil, errors.New("not a HEADERS frame")
 	}
-	// TODO: check size
+	if hf.Length > c.maxHeaderBytes() {
+		return nil, fmt.Errorf("Headers frame too large: %d bytes (max: %d)", hf.Length, c.maxHeaderBytes())
+	}
 	headerBlock := make([]byte, hf.Length)
 	if _, err := io.ReadFull(str, headerBlock); err != nil {
 		return nil, err

http3/client_test.go

@@ -31,7 +31,7 @@ var _ = Describe("Client", func() {
 	BeforeEach(func() {
 		origDialAddr = dialAddr
 		hostname := "quic.clemente.io:1337"
-		client = newClient(hostname, nil, &roundTripperOpts{}, nil, nil)
+		client = newClient(hostname, nil, &roundTripperOpts{MaxHeaderBytes: 1337}, nil, nil)
 		Expect(client.hostname).To(Equal(hostname))
 
 		var err error
@@ -275,6 +275,28 @@ var _ = Describe("Client", func() {
 				_, err := client.RoundTrip(request)
 				Expect(err).To(MatchError("test done"))
 			})
+
+			It("errors when the first frame is not a HEADERS frame", func() {
+				buf := &bytes.Buffer{}
+				(&dataFrame{Length: 0x42}).Write(buf)
+				str.EXPECT().Close().MaxTimes(1)
+				str.EXPECT().Read(gomock.Any()).DoAndReturn(func(b []byte) (int, error) {
+					return buf.Read(b)
+				}).AnyTimes()
+				_, err := client.RoundTrip(request)
+				Expect(err).To(MatchError("not a HEADERS frame"))
+			})
+
+			It("errors when the first frame is not a HEADERS frame", func() {
+				buf := &bytes.Buffer{}
+				(&headersFrame{Length: 1338}).Write(buf)
+				str.EXPECT().Close().MaxTimes(1)
+				str.EXPECT().Read(gomock.Any()).DoAndReturn(func(b []byte) (int, error) {
+					return buf.Read(b)
+				}).AnyTimes()
+				_, err := client.RoundTrip(request)
+				Expect(err).To(MatchError("Headers frame too large: 1338 bytes (max: 1337)"))
+			})
 		})
 
 		Context("gzip compression", func() {

http3/roundtrip.go

@@ -46,6 +46,11 @@ type RoundTripper struct {
 	// If Dial is nil, quic.DialAddr will be used.
 	Dial func(network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.Session, error)
 
+	// MaxResponseHeaderBytes specifies a limit on how many response bytes are
+	// allowed in the server's response header.
+	// Zero means to use a default limit.
+	MaxResponseHeaderBytes int64
+
 	clients map[string]roundTripCloser
 }
 
@@ -128,7 +133,10 @@ func (r *RoundTripper) getClient(hostname string, onlyCached bool) (http.RoundTr
 		client = newClient(
 			hostname,
 			r.TLSClientConfig,
-			&roundTripperOpts{DisableCompression: r.DisableCompression},
+			&roundTripperOpts{
+				DisableCompression: r.DisableCompression,
+				MaxHeaderBytes:     r.MaxResponseHeaderBytes,
+			},
 			r.QuicConfig,
 			r.Dial,
 		)

http3/server.go

@@ -155,6 +155,13 @@ func (s *Server) handleConn(sess quic.Session) {
 	}
 }
 
+func (s *Server) maxHeaderBytes() uint64 {
+	if s.Server.MaxHeaderBytes <= 0 {
+		return http.DefaultMaxHeaderBytes
+	}
+	return uint64(s.Server.MaxHeaderBytes)
+}
+
 // TODO: improve error handling.
 // Most (but not all) of the errors occurring here are connection-level erros.
 func (s *Server) handleRequest(str quic.Stream, decoder *qpack.Decoder) error {
@@ -168,7 +175,10 @@ func (s *Server) handleRequest(str quic.Stream, decoder *qpack.Decoder) error {
 		str.CancelWrite(quic.ErrorCode(errorUnexpectedFrame))
 		return errors.New("expected first frame to be a headers frame")
 	}
-	// TODO: check length
+	if hf.Length > s.maxHeaderBytes() {
+		str.CancelWrite(quic.ErrorCode(errorLimitExceeded))
+		return fmt.Errorf("Headers frame too large: %d bytes (max: %d)", hf.Length, s.maxHeaderBytes())
+	}
 	headerBlock := make([]byte, hf.Length)
 	if _, err := io.ReadFull(str, headerBlock); err != nil {
 		str.CancelWrite(quic.ErrorCode(errorIncompleteRequest))

http3/server_test.go

@@ -181,6 +181,28 @@ var _ = Describe("Server", func() {
 			Expect(hfs).To(HaveKeyWithValue(":status", []string{"200"}))
 		})
 
+		It("errors when the client sends a too large header frame", func() {
+			s.Server.MaxHeaderBytes = 42
+			s.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				Fail("Handler should not be called.")
+			})
+
+			requestData := encodeRequest(exampleGetRequest)
+			buf := &bytes.Buffer{}
+			(&dataFrame{Length: 6}).Write(buf) // add a body
+			buf.Write([]byte("foobar"))
+			responseBuf := &bytes.Buffer{}
+			setRequest(append(requestData, buf.Bytes()...))
+			str.EXPECT().Write(gomock.Any()).DoAndReturn(func(p []byte) (int, error) {
+				return responseBuf.Write(p)
+			}).AnyTimes()
+
+			str.EXPECT().CancelWrite(quic.ErrorCode(errorLimitExceeded))
+			err := s.handleRequest(str, qpackDecoder)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("Headers frame too large"))
+		})
+
 		It("cancels reading when the body of POST request is not read", func() {
 			handlerCalled := make(chan struct{})
 			s.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {