From bb4cfe29cb7dcdd99d860062e768a7e99c9491e1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 17 Aug 2019 10:53:32 +0700 Subject: [PATCH 1/2] fix connection ID length check in the NEW_CONNECTION_ID frame --- internal/wire/new_connection_id_frame.go | 4 ++-- internal/wire/new_connection_id_frame_test.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/wire/new_connection_id_frame.go b/internal/wire/new_connection_id_frame.go index ea472e41..7c53149c 100644 --- a/internal/wire/new_connection_id_frame.go +++ b/internal/wire/new_connection_id_frame.go @@ -35,7 +35,7 @@ func parseNewConnectionIDFrame(r *bytes.Reader, _ protocol.VersionNumber) (*NewC if err != nil { return nil, err } - if connIDLen < 4 || connIDLen > 18 { + if connIDLen > protocol.MaxConnIDLen { return nil, fmt.Errorf("invalid connection ID length: %d", connIDLen) } connID, err := protocol.ReadConnectionID(r, int(connIDLen)) @@ -62,7 +62,7 @@ func (f *NewConnectionIDFrame) Write(b *bytes.Buffer, _ protocol.VersionNumber) utils.WriteVarInt(b, f.SequenceNumber) utils.WriteVarInt(b, f.RetirePriorTo) connIDLen := f.ConnectionID.Len() - if connIDLen < 4 || connIDLen > 18 { + if connIDLen > protocol.MaxConnIDLen { return fmt.Errorf("invalid connection ID length: %d", connIDLen) } b.WriteByte(uint8(connIDLen)) diff --git a/internal/wire/new_connection_id_frame_test.go b/internal/wire/new_connection_id_frame_test.go index 5a299918..d4bcb829 100644 --- a/internal/wire/new_connection_id_frame_test.go +++ b/internal/wire/new_connection_id_frame_test.go @@ -30,14 +30,14 @@ var _ = Describe("NEW_CONNECTION_ID frame", func() { It("errors when the connection ID has an invalid length", func() { data := []byte{0x18} - data = append(data, encodeVarInt(0xdeadbeef)...) // sequence number - data = append(data, encodeVarInt(0xcafe)...) // retire prior to - data = append(data, 3) // connection ID length - data = append(data, []byte{1, 2, 3}...) // connection ID - data = append(data, []byte("deadbeefdecafbad")...) // stateless reset token + data = append(data, encodeVarInt(0xdeadbeef)...) // sequence number + data = append(data, encodeVarInt(0xcafe)...) // retire prior to + data = append(data, 21) // connection ID length + data = append(data, []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}...) // connection ID + data = append(data, []byte("deadbeefdecafbad")...) // stateless reset token b := bytes.NewReader(data) _, err := parseNewConnectionIDFrame(b, versionIETFFrames) - Expect(err).To(MatchError("invalid connection ID length: 3")) + Expect(err).To(MatchError("invalid connection ID length: 21")) }) It("errors on EOFs", func() { From 6bcd740f560f29d76760e2653d43a26a4cb031fb Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 17 Aug 2019 11:07:38 +0700 Subject: [PATCH 2/2] reject NEW_CONNECTION_ID frames with invalid Retire Prior To values --- internal/wire/new_connection_id_frame.go | 3 +++ internal/wire/new_connection_id_frame_test.go | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/internal/wire/new_connection_id_frame.go b/internal/wire/new_connection_id_frame.go index 7c53149c..59094744 100644 --- a/internal/wire/new_connection_id_frame.go +++ b/internal/wire/new_connection_id_frame.go @@ -31,6 +31,9 @@ func parseNewConnectionIDFrame(r *bytes.Reader, _ protocol.VersionNumber) (*NewC if err != nil { return nil, err } + if ret > seq { + return nil, fmt.Errorf("Retire Prior To value (%d) larger than Sequence Number (%d)", ret, seq) + } connIDLen, err := r.ReadByte() if err != nil { return nil, err diff --git a/internal/wire/new_connection_id_frame_test.go b/internal/wire/new_connection_id_frame_test.go index d4bcb829..c4cc0807 100644 --- a/internal/wire/new_connection_id_frame_test.go +++ b/internal/wire/new_connection_id_frame_test.go @@ -28,6 +28,18 @@ var _ = Describe("NEW_CONNECTION_ID frame", func() { Expect(string(frame.StatelessResetToken[:])).To(Equal("deadbeefdecafbad")) }) + It("errors when the Retire Prior To value is larger than the Sequence Number", func() { + data := []byte{0x18} + data = append(data, encodeVarInt(1000)...) // sequence number + data = append(data, encodeVarInt(1001)...) // retire prior to + data = append(data, 3) + data = append(data, []byte{1, 2, 3}...) + data = append(data, []byte("deadbeefdecafbad")...) // stateless reset token + b := bytes.NewReader(data) + _, err := parseNewConnectionIDFrame(b, versionIETFFrames) + Expect(err).To(MatchError("Retire Prior To value (1001) larger than Sequence Number (1000)")) + }) + It("errors when the connection ID has an invalid length", func() { data := []byte{0x18} data = append(data, encodeVarInt(0xdeadbeef)...) // sequence number