From fe2abcb6e15f7a34d285220b437d421da4c76775 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 29 Aug 2023 08:21:51 -0700 Subject: [PATCH] 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 Reviewed-by: Jonathan Amsterdam --- internal/quic/packet_parser.go | 2 +- internal/quic/quic.go | 4 ++++ internal/quic/stream_test.go | 4 ++-- internal/quic/transport_params.go | 6 ++++++ internal/quic/transport_params_test.go | 14 ++++++++++++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/internal/quic/packet_parser.go b/internal/quic/packet_parser.go index 9a00da7560..ca5b37b2bd 100644 --- 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 index 8cd61aed08..71738e129d 100644 --- 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 index bafd236c9f..7b8ba2c54f 100644 --- 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 index 89ea69fb97..dc76d16509 100644 --- 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 index e1c45ca0e6..cc88e83fd6 100644 --- 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{