From a4773eb5ff1c9f03f3d667c05c21b73c818d299b Mon Sep 17 00:00:00 2001
From: Lucas Clemente <lucas@clemente.io>
Date: Fri, 27 May 2016 23:25:51 +0200
Subject: [PATCH] validate length of crypto message

refs #123
---
 handshake/handshake_message.go      | 14 ++++++++++++--
 handshake/handshake_message_test.go | 18 ++++++++++++++++++
 protocol/server_parameters.go       |  7 +++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/handshake/handshake_message.go b/handshake/handshake_message.go
index 8e4cae5f1..32e8aa58e 100644
--- a/handshake/handshake_message.go
+++ b/handshake/handshake_message.go
@@ -7,6 +7,8 @@ import (
 	"io"
 	"sort"
 
+	"github.com/lucas-clemente/quic-go/protocol"
+	"github.com/lucas-clemente/quic-go/qerr"
 	"github.com/lucas-clemente/quic-go/utils"
 )
 
@@ -22,6 +24,10 @@ func ParseHandshakeMessage(r utils.ReadStream) (Tag, map[Tag][]byte, error) {
 		return 0, nil, err
 	}
 
+	if nPairs > protocol.CryptoMaxParams {
+		return 0, nil, qerr.CryptoTooManyEntries
+	}
+
 	index := make([]byte, nPairs*8)
 	_, err = io.ReadFull(r, index)
 	if err != nil {
@@ -32,11 +38,15 @@ func ParseHandshakeMessage(r utils.ReadStream) (Tag, map[Tag][]byte, error) {
 
 	dataStart := 0
 	for indexPos := 0; indexPos < int(nPairs)*8; indexPos += 8 {
-		// We know from the check above that data is long enough for the index
 		tag := Tag(binary.LittleEndian.Uint32(index[indexPos : indexPos+4]))
 		dataEnd := int(binary.LittleEndian.Uint32(index[indexPos+4 : indexPos+8]))
 
-		data := make([]byte, dataEnd-dataStart)
+		dataLen := dataEnd - dataStart
+		if dataLen > protocol.CryptoParameterMaxLength {
+			return 0, nil, qerr.Error(qerr.CryptoInvalidValueLength, "value too long")
+		}
+
+		data := make([]byte, dataLen)
 		_, err = io.ReadFull(r, data)
 		if err != nil {
 			return 0, nil, err
diff --git a/handshake/handshake_message_test.go b/handshake/handshake_message_test.go
index cbf2476b5..81eac3173 100644
--- a/handshake/handshake_message_test.go
+++ b/handshake/handshake_message_test.go
@@ -3,6 +3,7 @@ package handshake
 import (
 	"bytes"
 
+	"github.com/lucas-clemente/quic-go/qerr"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -15,6 +16,23 @@ var _ = Describe("Handshake Message", func() {
 			Expect(tag).To(Equal(TagCHLO))
 			Expect(msg).To(Equal(sampleCHLOMap))
 		})
+
+		It("rejects large numbers of pairs", func() {
+			r := bytes.NewReader([]byte("CHLO\xff\xff\xff\xff"))
+			_, _, err := ParseHandshakeMessage(r)
+			Expect(err).To(MatchError(qerr.CryptoTooManyEntries))
+		})
+
+		It("rejects too long values", func() {
+			r := bytes.NewReader([]byte{
+				'C', 'H', 'L', 'O',
+				1, 0, 0, 0,
+				0, 0, 0, 0,
+				0xff, 0xff, 0xff, 0xff,
+			})
+			_, _, err := ParseHandshakeMessage(r)
+			Expect(err).To(MatchError(qerr.Error(qerr.CryptoInvalidValueLength, "value too long")))
+		})
 	})
 
 	Context("when writing", func() {
diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go
index 8fb250ae9..421e2f445 100644
--- a/protocol/server_parameters.go
+++ b/protocol/server_parameters.go
@@ -58,3 +58,10 @@ const STKExpiryTimeSec = 24 * 60 * 60
 // TODO: find a reasonable value here
 // TODO: decrease this value after dropping support for QUIC 33 and earlier
 const MaxTrackedSentPackets uint32 = 2000
+
+// CryptoMaxParams is the upper limit for the number of parameters in a crypto message.
+// Value taken from Chrome.
+const CryptoMaxParams = 128
+
+// CryptoParameterMaxLength is the upper limit for the length of a parameter in a crypto message.
+const CryptoParameterMaxLength = ClientHelloMinimumSize