Merge pull request #1840 from lucas-clemente/invalid-max-streams-values

error on invalid maximum stream number values

internal/protocol/protocol.go

@@ -64,6 +64,10 @@ const MinStatelessResetSize = 1 /* first byte */ + 22 /* random bytes */ + 16 /*
 // MinConnectionIDLenInitial is the minimum length of the destination connection ID on an Initial packet.
 const MinConnectionIDLenInitial = 8
 
+// MaxStreamCount is the maximum stream count value that can be sent in MAX_STREAMS frames
+// and as the stream count in the transport parameters
+const MaxStreamCount = 1 << 60
+
 // DefaultAckDelayExponent is the default ack delay exponent
 const DefaultAckDelayExponent = 3
 

mock_stream_manager_test.go

@@ -197,9 +197,11 @@ func (mr *MockStreamManagerMockRecorder) OpenUniStreamSync() *gomock.Call {
 }
 
 // UpdateLimits mocks base method
-func (m *MockStreamManager) UpdateLimits(arg0 *handshake.TransportParameters) {
+func (m *MockStreamManager) UpdateLimits(arg0 *handshake.TransportParameters) error {
 	m.ctrl.T.Helper()
-	m.ctrl.Call(m, "UpdateLimits", arg0)
+	ret := m.ctrl.Call(m, "UpdateLimits", arg0)
+	ret0, _ := ret[0].(error)
+	return ret0
 }
 
 // UpdateLimits indicates an expected call of UpdateLimits

session.go

@@ -41,7 +41,7 @@ type streamManager interface {
 	AcceptStream() (Stream, error)
 	AcceptUniStream() (ReceiveStream, error)
 	DeleteStream(protocol.StreamID) error
-	UpdateLimits(*handshake.TransportParameters)
+	UpdateLimits(*handshake.TransportParameters) error
 	HandleMaxStreamsFrame(*wire.MaxStreamsFrame) error
 	CloseWithError(error)
 }
@@ -932,7 +932,10 @@ func (s *session) processTransportParameters(data []byte) {
 	}
 	s.logger.Debugf("Received Transport Parameters: %s", params)
 	s.peerParams = params
-	s.streamsMap.UpdateLimits(params)
+	if err := s.streamsMap.UpdateLimits(params); err != nil {
+		s.closeLocal(err)
+		return
+	}
 	s.packer.HandleTransportParameters(params)
 	s.frameParser.SetAckDelayExponent(params.AckDelayExponent)
 	s.connFlowController.UpdateSendWindow(params.InitialMaxData)

streams_map.go

@@ -8,6 +8,7 @@ import (
 	"github.com/lucas-clemente/quic-go/internal/flowcontrol"
 	"github.com/lucas-clemente/quic-go/internal/handshake"
 	"github.com/lucas-clemente/quic-go/internal/protocol"
+	"github.com/lucas-clemente/quic-go/internal/qerr"
 	"github.com/lucas-clemente/quic-go/internal/wire"
 )
 
@@ -159,6 +160,9 @@ func (m *streamsMap) GetOrOpenSendStream(id protocol.StreamID) (sendStreamI, err
 }
 
 func (m *streamsMap) HandleMaxStreamsFrame(f *wire.MaxStreamsFrame) error {
+	if f.MaxStreams > protocol.MaxStreamCount {
+		return qerr.StreamLimitError
+	}
 	id := protocol.MaxStreamID(f.Type, f.MaxStreams, m.perspective)
 	switch id.Type() {
 	case protocol.StreamTypeUni:
@@ -170,10 +174,14 @@ func (m *streamsMap) HandleMaxStreamsFrame(f *wire.MaxStreamsFrame) error {
 	return nil
 }
 
-func (m *streamsMap) UpdateLimits(p *handshake.TransportParameters) {
+func (m *streamsMap) UpdateLimits(p *handshake.TransportParameters) error {
+	if p.MaxBidiStreams > protocol.MaxStreamCount || p.MaxUniStreams > protocol.MaxStreamCount {
+		return qerr.StreamLimitError
+	}
 	// Max{Uni,Bidi}StreamID returns the highest stream ID that the peer is allowed to open.
 	m.outgoingBidiStreams.SetMaxStream(protocol.MaxStreamID(protocol.StreamTypeBidi, p.MaxBidiStreams, m.perspective))
 	m.outgoingUniStreams.SetMaxStream(protocol.MaxStreamID(protocol.StreamTypeUni, p.MaxUniStreams, m.perspective))
+	return nil
 }
 
 func (m *streamsMap) CloseWithError(err error) {

streams_map_test.go

@@ -295,33 +295,43 @@ var _ = Describe("Streams Map", func() {
 			})
 
 			Context("updating stream ID limits", func() {
-				BeforeEach(func() {
-					mockSender.EXPECT().queueControlFrame(gomock.Any())
-				})
-
 				It("processes the parameter for outgoing streams, as a server", func() {
+					mockSender.EXPECT().queueControlFrame(gomock.Any())
 					m.perspective = protocol.PerspectiveServer
 					_, err := m.OpenStream()
 					expectTooManyStreamsError(err)
-					m.UpdateLimits(&handshake.TransportParameters{
+					Expect(m.UpdateLimits(&handshake.TransportParameters{
 						MaxBidiStreams: 5,
 						MaxUniStreams:  5,
-					})
+					})).To(Succeed())
 					Expect(m.outgoingBidiStreams.maxStream).To(Equal(protocol.StreamID(17)))
 					Expect(m.outgoingUniStreams.maxStream).To(Equal(protocol.StreamID(19)))
 				})
 
 				It("processes the parameter for outgoing streams, as a client", func() {
+					mockSender.EXPECT().queueControlFrame(gomock.Any())
 					m.perspective = protocol.PerspectiveClient
 					_, err := m.OpenUniStream()
 					expectTooManyStreamsError(err)
-					m.UpdateLimits(&handshake.TransportParameters{
+					Expect(m.UpdateLimits(&handshake.TransportParameters{
 						MaxBidiStreams: 5,
 						MaxUniStreams:  5,
-					})
+					})).To(Succeed())
 					Expect(m.outgoingBidiStreams.maxStream).To(Equal(protocol.StreamID(16)))
 					Expect(m.outgoingUniStreams.maxStream).To(Equal(protocol.StreamID(18)))
 				})
+
+				It("rejects parameters with too large unidirectional stream counts", func() {
+					Expect(m.UpdateLimits(&handshake.TransportParameters{
+						MaxUniStreams: protocol.MaxStreamCount + 1,
+					})).To(MatchError(qerr.StreamLimitError))
+				})
+
+				It("rejects parameters with too large unidirectional stream counts", func() {
+					Expect(m.UpdateLimits(&handshake.TransportParameters{
+						MaxBidiStreams: protocol.MaxStreamCount + 1,
+					})).To(MatchError(qerr.StreamLimitError))
+				})
 			})
 
 			Context("handling MAX_STREAMS frames", func() {
@@ -356,6 +366,13 @@ var _ = Describe("Streams Map", func() {
 					_, err = m.OpenUniStream()
 					expectTooManyStreamsError(err)
 				})
+
+				It("rejects MAX_STREAMS frames with too large values", func() {
+					Expect(m.HandleMaxStreamsFrame(&wire.MaxStreamsFrame{
+						Type:       protocol.StreamTypeBidi,
+						MaxStreams: protocol.MaxStreamCount + 1,
+					})).To(MatchError(qerr.StreamLimitError))
+				})
 			})
 
 			Context("sending MAX_STREAMS frames", func() {