limit number of ACK ranges in ReceivedPacketHistory

fixes #195

ackhandler/received_packet_handler.go

@@ -21,7 +21,7 @@ var (
 
 var (
 	errInvalidPacketNumber               = errors.New("ReceivedPacketHandler: Invalid packet number")
-	errTooManyOutstandingReceivedPackets = qerr.Error(qerr.TooManyOutstandingReceivedPackets, "")
+	errTooManyOutstandingReceivedPackets = qerr.Error(qerr.TooManyOutstandingReceivedPackets, "Too many outstanding received packets")
 )
 
 type receivedPacketHandler struct {
@@ -61,7 +61,10 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
 		return ErrDuplicatePacket
 	}
 
-	h.packetHistory.ReceivedPacket(packetNumber)
+	err := h.packetHistory.ReceivedPacket(packetNumber)
+	if err != nil {
+		return err
+	}
 
 	h.stateChanged = true
 	h.currentAckFrame = nil

ackhandler/received_packet_handler_test.go

@@ -83,14 +83,28 @@ var _ = Describe("receivedPacketHandler", func() {
 		})
 
 		It("doesn't store more than MaxTrackedReceivedPackets packets", func() {
-			for i := protocol.PacketNumber(0); i < protocol.MaxTrackedReceivedPackets; i++ {
-				packetNumber := protocol.PacketNumber(1 + 2*i)
-				err := handler.ReceivedPacket(packetNumber)
+			err := handler.ReceivedPacket(1)
+			Expect(err).ToNot(HaveOccurred())
+			for i := protocol.PacketNumber(3); i < 3+protocol.MaxTrackedReceivedPackets-1; i++ {
+				err := handler.ReceivedPacket(protocol.PacketNumber(i))
 				Expect(err).ToNot(HaveOccurred())
 			}
-			err := handler.ReceivedPacket(protocol.PacketNumber(3 * protocol.MaxTrackedReceivedPackets))
+			err = handler.ReceivedPacket(protocol.PacketNumber(protocol.MaxTrackedReceivedPackets) + 10)
 			Expect(err).To(MatchError(errTooManyOutstandingReceivedPackets))
 		})
+
+		It("passes on errors from receivedPacketHistory", func() {
+			var err error
+			for i := protocol.PacketNumber(0); i < 5*protocol.MaxTrackedReceivedAckRanges; i++ {
+				err = handler.ReceivedPacket(2*i + 1)
+				// this will eventually return an error
+				// details about when exactly the receivedPacketHistory errors are tested there
+				if err != nil {
+					break
+				}
+			}
+			Expect(err).To(MatchError(errTooManyOutstandingReceivedAckRanges))
+		})
 	})
 
 	Context("handling STOP_WAITING frames", func() {

ackhandler/received_packet_history.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/lucas-clemente/quic-go/frames"
 	"github.com/lucas-clemente/quic-go/protocol"
+	"github.com/lucas-clemente/quic-go/qerr"
 	"github.com/lucas-clemente/quic-go/utils"
 )
 
@@ -14,6 +15,8 @@ type receivedPacketHistory struct {
 	mutex sync.RWMutex
 }
 
+var errTooManyOutstandingReceivedAckRanges = qerr.Error(qerr.TooManyOutstandingReceivedPackets, "Too many outstanding received ACK ranges")
+
 // newReceivedPacketHistory creates a new received packet history
 func newReceivedPacketHistory() *receivedPacketHistory {
 	return &receivedPacketHistory{
@@ -22,19 +25,23 @@ func newReceivedPacketHistory() *receivedPacketHistory {
 }
 
 // ReceivedPacket registers a packet with PacketNumber p and updates the ranges
-func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) {
+func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) error {
 	h.mutex.Lock()
 	defer h.mutex.Unlock()
 
+	if h.ranges.Len() >= protocol.MaxTrackedReceivedAckRanges {
+		return errTooManyOutstandingReceivedAckRanges
+	}
+
 	if h.ranges.Len() == 0 {
 		h.ranges.PushBack(utils.PacketInterval{Start: p, End: p})
-		return
+		return nil
 	}
 
 	for el := h.ranges.Back(); el != nil; el = el.Prev() {
 		// p already included in an existing range. Nothing to do here
 		if p >= el.Value.Start && p <= el.Value.End {
-			return
+			return nil
 		}
 
 		var rangeExtended bool
@@ -52,20 +59,22 @@ func (h *receivedPacketHistory) ReceivedPacket(p protocol.PacketNumber) {
 			if prev != nil && prev.Value.End+1 == el.Value.Start { // merge two ranges
 				prev.Value.End = el.Value.End
 				h.ranges.Remove(el)
-				return
+				return nil
 			}
-			return // if the two ranges were not merge, we're done here
+			return nil // if the two ranges were not merge, we're done here
 		}
 
 		// create a new range at the end
 		if p > el.Value.End {
 			h.ranges.InsertAfter(utils.PacketInterval{Start: p, End: p}, el)
-			return
+			return nil
 		}
 	}
 
 	// create a new range at the beginning
 	h.ranges.InsertBefore(utils.PacketInterval{Start: p, End: p}, h.ranges.Front())
+
+	return nil
 }
 
 func (h *receivedPacketHistory) DeleteBelow(leastUnacked protocol.PacketNumber) {

ackhandler/received_packet_history_test.go

@@ -2,6 +2,7 @@ package ackhandler
 
 import (
 	"github.com/lucas-clemente/quic-go/frames"
+	"github.com/lucas-clemente/quic-go/protocol"
 	"github.com/lucas-clemente/quic-go/utils"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -171,6 +172,29 @@ var _ = Describe("receivedPacketHistory", func() {
 			Expect(hist.ranges.Len()).To(Equal(1))
 			Expect(hist.ranges.Front().Value).To(Equal(utils.PacketInterval{Start: 4, End: 4}))
 		})
+
+		Context("DoS protection", func() {
+			It("doesn't create more than MaxTrackedReceivedAckRanges ranges", func() {
+				for i := protocol.PacketNumber(1); i <= protocol.MaxTrackedReceivedAckRanges; i++ {
+					err := hist.ReceivedPacket(2 * i)
+					Expect(err).ToNot(HaveOccurred())
+				}
+				err := hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 2)
+				Expect(err).To(MatchError(errTooManyOutstandingReceivedAckRanges))
+			})
+
+			It("doesn't consider already deleted ranges for MaxTrackedReceivedAckRanges", func() {
+				for i := protocol.PacketNumber(1); i <= protocol.MaxTrackedReceivedAckRanges; i++ {
+					err := hist.ReceivedPacket(2 * i)
+					Expect(err).ToNot(HaveOccurred())
+				}
+				err := hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 2)
+				Expect(err).To(MatchError(errTooManyOutstandingReceivedAckRanges))
+				hist.DeleteBelow(protocol.MaxTrackedReceivedAckRanges) // deletes about half of the ranges
+				err = hist.ReceivedPacket(2*protocol.MaxTrackedReceivedAckRanges + 4)
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
 	})
 
 	Context("ACK range export", func() {