Guard against oversized SPDY frames

Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Author: dims

connection.go

@@ -350,6 +350,9 @@ Loop:
 			} else {
 				debugMessage("(%p) EOF received", s)
 			}
+			if spdyErr, ok := err.(*spdy.Error); ok && spdyErr.Err == spdy.InvalidControlFrame {
+				_ = s.conn.Close()
+			}
 			break
 		}
 		var priority uint8

spdy/read.go

@@ -43,6 +43,11 @@ func (frame *SettingsFrame) read(h ControlFrameHeader, f *Framer) error {
 	if err := binary.Read(f.r, binary.BigEndian, &numSettings); err != nil {
 		return err
 	}
+	// Each setting is 8 bytes (4-byte id + 4-byte value).
+	// Payload is 4 bytes for numSettings + numSettings*8.
+	if h.length < 4 || numSettings > (h.length-4)/8 {
+		return &Error{InvalidControlFrame, 0}
+	}
 	frame.FlagIdValues = make([]SettingsFlagIdValue, numSettings)
 	for i := uint32(0); i < numSettings; i++ {
 		if err := binary.Read(f.r, binary.BigEndian, &frame.FlagIdValues[i].Id); err != nil {
@@ -161,8 +166,19 @@ func (f *Framer) parseControlFrame(version uint16, frameType ControlFrameType) (
 	if err := binary.Read(f.r, binary.BigEndian, &length); err != nil {
 		return nil, err
 	}
+	maxControlFramePayload := uint32(MaxDataLength)
+	if f.maxFrameLength > 0 {
+		maxControlFramePayload = f.maxFrameLength
+	}
+
 	flags := ControlFlags((length & 0xff000000) >> 24)
 	length &= 0xffffff
+	if length > maxControlFramePayload {
+		if _, err := io.CopyN(io.Discard, f.r, int64(length)); err != nil {
+			return nil, err
+		}
+		return nil, &Error{InvalidControlFrame, 0}
+	}
 	header := ControlFrameHeader{version, frameType, flags, length}
 	cframe, err := newControlFrame(frameType)
 	if err != nil {
@@ -174,7 +190,7 @@ func (f *Framer) parseControlFrame(version uint16, frameType ControlFrameType) (
 	return cframe, nil
 }
 
-func parseHeaderValueBlock(r io.Reader, streamId StreamId) (http.Header, error) {
+func (f *Framer) parseHeaderValueBlock(r io.Reader, streamId StreamId) (http.Header, error) {
 	var numHeaders uint32
 	if err := binary.Read(r, binary.BigEndian, &numHeaders); err != nil {
 		return nil, err
@@ -240,7 +256,7 @@ func (f *Framer) readSynStreamFrame(h ControlFrameHeader, frame *SynStreamFrame)
 		}
 		reader = f.headerDecompressor
 	}
-	frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId)
+	frame.Headers, err = f.parseHeaderValueBlock(reader, frame.StreamId)
 	if !f.headerCompressionDisabled && (err == io.EOF && f.headerReader.N == 0 || f.headerReader.N != 0) {
 		err = &Error{WrongCompressedPayloadSize, 0}
 	}
@@ -272,7 +288,7 @@ func (f *Framer) readSynReplyFrame(h ControlFrameHeader, frame *SynReplyFrame) e
 		}
 		reader = f.headerDecompressor
 	}
-	frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId)
+	frame.Headers, err = f.parseHeaderValueBlock(reader, frame.StreamId)
 	if !f.headerCompressionDisabled && (err == io.EOF && f.headerReader.N == 0 || f.headerReader.N != 0) {
 		err = &Error{WrongCompressedPayloadSize, 0}
 	}
@@ -304,7 +320,7 @@ func (f *Framer) readHeadersFrame(h ControlFrameHeader, frame *HeadersFrame) err
 		}
 		reader = f.headerDecompressor
 	}
-	frame.Headers, err = parseHeaderValueBlock(reader, frame.StreamId)
+	frame.Headers, err = f.parseHeaderValueBlock(reader, frame.StreamId)
 	if !f.headerCompressionDisabled && (err == io.EOF && f.headerReader.N == 0 || f.headerReader.N != 0) {
 		err = &Error{WrongCompressedPayloadSize, 0}
 	}

spdy/spdy_test.go

@@ -27,7 +27,8 @@ func TestHeaderParsing(t *testing.T) {
 	var headerValueBlockBuf bytes.Buffer
 	writeHeaderValueBlock(&headerValueBlockBuf, HeadersFixture)
 	const bogusStreamId = 1
-	newHeaders, err := parseHeaderValueBlock(&headerValueBlockBuf, bogusStreamId)
+	f := &Framer{}
+	newHeaders, err := f.parseHeaderValueBlock(&headerValueBlockBuf, bogusStreamId)
 	if err != nil {
 		t.Fatal("parseHeaderValueBlock:", err)
 	}
@@ -223,6 +224,51 @@ func TestCreateParseSettings(t *testing.T) {
 	}
 }
 
+func TestReadFrameRejectsOversizedControlFrame(t *testing.T) {
+	buffer := new(bytes.Buffer)
+	framer, err := NewFramer(buffer, buffer)
+	if err != nil {
+		t.Fatal("Failed to create new framer:", err)
+	}
+	framer.maxFrameLength = 1
+
+	// Control frame header (version 3, SETTINGS, length = 2)
+	_, _ = buffer.Write([]byte{
+		0x80, 0x03, // control frame + version
+		0x00, 0x04, // SETTINGS
+		0x00,             // flags
+		0x00, 0x00, 0x02, // length = 2
+		0x00, 0x01, // payload (2 bytes)
+	})
+
+	_, err = framer.ReadFrame()
+	if err == nil {
+		t.Fatal("expected error for oversized control frame")
+	}
+}
+
+func TestReadFrameRejectsSettingsLengthMismatch(t *testing.T) {
+	buffer := new(bytes.Buffer)
+	framer, err := NewFramer(buffer, buffer)
+	if err != nil {
+		t.Fatal("Failed to create new framer:", err)
+	}
+
+	// SETTINGS frame with declared payload length 0, but includes numSettings.
+	_, _ = buffer.Write([]byte{
+		0x80, 0x03, // control frame + version
+		0x00, 0x04, // SETTINGS
+		0x00,             // flags
+		0x00, 0x00, 0x00, // length = 0
+		0x00, 0x00, 0x00, 0x01, // numSettings = 1
+	})
+
+	_, err = framer.ReadFrame()
+	if err == nil {
+		t.Fatal("expected error for SETTINGS length mismatch")
+	}
+}
+
 func TestCreateParsePing(t *testing.T) {
 	buffer := new(bytes.Buffer)
 	framer, err := NewFramer(buffer, buffer)

spdy/types.go

@@ -255,6 +255,8 @@ type Framer struct {
 	r                         io.Reader
 	headerReader              io.LimitedReader
 	headerDecompressor        io.ReadCloser
+
+	maxFrameLength uint32 // overrides the default frame payload length limit.
 }
 
 // NewFramer allocates a new Framer for a given SPDY connection, represented by