quic: validate stream limits in transport params

The maximum number of streams of a given type (bidi/uni) is capped to 2^60, since a larger number would overflow a varint. Validate limits received in transport parameters. RFC 9000, Section 4.6 For golang/go#58547 Change-Id: I7a4a15c569da91ad1b89a5dc71e1c5b213dbda9a Reviewed-on: https://go-review.googlesource.com/c/net/+/524037 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Jonathan Amsterdam <[email protected]>
golang · Aug 29, 2023 · fe2abcb · fe2abcb
1 parent d1b0a97
commit fe2abcb
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 3 deletions.
diff --git a/internal/quic/packet_parser.go b/internal/quic/packet_parser.go
@@ -378,7 +378,7 @@ func consumeMaxStreamsFrame(b []byte) (typ streamType, max int64, n int) {
 		return 0, 0, -1
 	}
 	n += nn
-	if v > 1<<60 {
+	if v > maxStreamsLimit {
 		return 0, 0, -1
 	}
 	return typ, int64(v), n

diff --git a/internal/quic/quic.go b/internal/quic/quic.go
@@ -55,6 +55,10 @@ const timerGranularity = 1 * time.Millisecond
 // https://www.rfc-editor.org/rfc/rfc9000#section-14.1
 const minimumClientInitialDatagramSize = 1200
 
+// Maximum number of streams of a given type which may be created.
+// https://www.rfc-editor.org/rfc/rfc9000.html#section-4.6-2
+const maxStreamsLimit = 1 << 60
+
 // A connSide distinguishes between the client and server sides of a connection.
 type connSide int8
 

diff --git a/internal/quic/stream_test.go b/internal/quic/stream_test.go
@@ -1165,8 +1165,8 @@ func newTestConnAndRemoteStream(t *testing.T, side connSide, styp streamType, op
 
 // permissiveTransportParameters may be passed as an option to newTestConn.
 func permissiveTransportParameters(p *transportParameters) {
-	p.initialMaxStreamsBidi = maxVarint
-	p.initialMaxStreamsUni = maxVarint
+	p.initialMaxStreamsBidi = maxStreamsLimit
+	p.initialMaxStreamsUni = maxStreamsLimit
 	p.initialMaxData = maxVarint
 	p.initialMaxStreamDataBidiRemote = maxVarint
 	p.initialMaxStreamDataBidiLocal = maxVarint

diff --git a/internal/quic/transport_params.go b/internal/quic/transport_params.go
@@ -212,8 +212,14 @@ func unmarshalTransportParams(params []byte) (transportParameters, error) {
 			p.initialMaxStreamDataUni, n = consumeVarintInt64(val)
 		case paramInitialMaxStreamsBidi:
 			p.initialMaxStreamsBidi, n = consumeVarintInt64(val)
+			if p.initialMaxStreamsBidi > maxStreamsLimit {
+				return p, localTransportError(errTransportParameter)
+			}
 		case paramInitialMaxStreamsUni:
 			p.initialMaxStreamsUni, n = consumeVarintInt64(val)
+			if p.initialMaxStreamsUni > maxStreamsLimit {
+				return p, localTransportError(errTransportParameter)
+			}
 		case paramAckDelayExponent:
 			var v uint64
 			v, n = consumeVarint(val)

diff --git a/internal/quic/transport_params_test.go b/internal/quic/transport_params_test.go
@@ -236,6 +236,20 @@ func TestTransportParametersErrors(t *testing.T) {
 			15,   // length
 			0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
 		},
+	}, {
+		desc: "initial_max_streams_bidi is too large",
+		enc: []byte{
+			0x08, // initial_max_streams_bidi,
+			8,    // length,
+			0xd0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+		},
+	}, {
+		desc: "initial_max_streams_uni is too large",
+		enc: []byte{
+			0x08, // initial_max_streams_uni,
+			9,    // length,
+			0xd0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+		},
 	}, {
 		desc: "preferred_address is too short",
 		enc: []byte{