From d476067f65d7019122ce50311f83fa19ba8c931d Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 19 Aug 2020 12:07:58 +0700 Subject: [PATCH] fix overflow of the max_ack_delay when parsing transport parameters --- internal/protocol/protocol.go | 2 +- internal/wire/transport_parameter_test.go | 8 ++++---- internal/wire/transport_parameters.go | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/protocol/protocol.go b/internal/protocol/protocol.go index aac729563..484bf4c2f 100644 --- a/internal/protocol/protocol.go +++ b/internal/protocol/protocol.go @@ -71,7 +71,7 @@ const MaxAckDelayExponent = 20 const DefaultMaxAckDelay = 25 * time.Millisecond // MaxMaxAckDelay is the maximum max_ack_delay -const MaxMaxAckDelay = 1 << 14 * time.Millisecond +const MaxMaxAckDelay = (1<<14 - 1) * time.Millisecond // MaxConnIDLen is the maximum length of the connection ID const MaxConnIDLen = 20 diff --git a/internal/wire/transport_parameter_test.go b/internal/wire/transport_parameter_test.go index 6a8f44ee1..4c2c067dd 100644 --- a/internal/wire/transport_parameter_test.go +++ b/internal/wire/transport_parameter_test.go @@ -248,16 +248,16 @@ var _ = Describe("Transport Parameters", func() { Expect(err.Error()).To(ContainSubstring("TRANSPORT_PARAMETER_ERROR: inconsistent transport parameter length")) }) - It("handles max_ack_delays that decode to a negative duration", func() { + It("handles huge max_ack_delay values", func() { b := &bytes.Buffer{} val := uint64(math.MaxUint64) / 5 utils.WriteVarInt(b, uint64(maxAckDelayParameterID)) utils.WriteVarInt(b, uint64(utils.VarIntLen(val))) utils.WriteVarInt(b, val) addInitialSourceConnectionID(b) - p := &TransportParameters{} - Expect(p.Unmarshal(b.Bytes(), protocol.PerspectiveClient)).To(Succeed()) - Expect(p.MaxAckDelay).To(BeNumerically(">", 290*365*24*time.Hour)) + err := (&TransportParameters{}).Unmarshal(b.Bytes(), protocol.PerspectiveClient) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid value for max_ack_delay")) }) It("skips unknown parameters", func() { diff --git a/internal/wire/transport_parameters.go b/internal/wire/transport_parameters.go index 4694cc178..bf3eca3a3 100644 --- a/internal/wire/transport_parameters.go +++ b/internal/wire/transport_parameters.go @@ -297,10 +297,10 @@ func (p *TransportParameters) readNumericTransportParameter( } p.AckDelayExponent = uint8(val) case maxAckDelayParameterID: - maxAckDelay := time.Duration(val) * time.Millisecond - if maxAckDelay >= protocol.MaxMaxAckDelay { - return fmt.Errorf("invalid value for max_ack_delay: %dms (maximum %dms)", maxAckDelay/time.Millisecond, (protocol.MaxMaxAckDelay-time.Millisecond)/time.Millisecond) + if val > uint64(protocol.MaxMaxAckDelay/time.Millisecond) { + return fmt.Errorf("invalid value for max_ack_delay: %dms (maximum %dms)", val, protocol.MaxMaxAckDelay/time.Millisecond) } + maxAckDelay := time.Duration(val) * time.Millisecond if maxAckDelay < 0 { maxAckDelay = utils.InfDuration }