From 1de32fba17a392b656c73a917843724b92b8989d Mon Sep 17 00:00:00 2001
From: Ismail Khoffi <Ismail.Khoffi@gmail.com>
Date: Sun, 2 Sep 2018 07:13:09 +0100
Subject: [PATCH] Check for int overflow in clist (#2289)

* explicitly panic if max capacity is reached

* address review comments

* comments and a test
---
 CHANGELOG_PENDING.md     |  1 +
 libs/clist/clist.go      | 23 ++++++++++++++++++++++-
 libs/clist/clist_test.go | 13 +++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
index aab9096a..30303008 100644
--- a/CHANGELOG_PENDING.md
+++ b/CHANGELOG_PENDING.md
@@ -22,6 +22,7 @@ BREAKING CHANGES:
     - Update field names and types in BeginBlock
 - [state] Implement BFT time    
 - [p2p] update secret connection to use a little endian encoded nonce
+- [libs/clist] Panics if list extends beyond MaxLength
 
 FEATURES:
 - [types] allow genesis file to have 0 validators ([#2015](https://github.com/tendermint/tendermint/issues/2015))
diff --git a/libs/clist/clist.go b/libs/clist/clist.go
index b3e66efc..c69d3d5f 100644
--- a/libs/clist/clist.go
+++ b/libs/clist/clist.go
@@ -12,9 +12,15 @@ to ensure garbage collection of removed elements.
 */
 
 import (
+	"fmt"
 	"sync"
 )
 
+// MaxLength is the max allowed number of elements a linked list is
+// allowed to contain.
+// If more elements are pushed to the list it will panic.
+const MaxLength = int(^uint(0) >> 1)
+
 /*
 
 CElement is an element of a linked-list
@@ -210,6 +216,7 @@ func (e *CElement) SetRemoved() {
 // CList represents a linked list.
 // The zero value for CList is an empty list ready to use.
 // Operations are goroutine-safe.
+// Panics if length grows beyond the max.
 type CList struct {
 	mtx    sync.RWMutex
 	wg     *sync.WaitGroup
@@ -217,6 +224,7 @@ type CList struct {
 	head   *CElement // first element
 	tail   *CElement // last element
 	len    int       // list length
+	maxLen int       // max list length
 }
 
 func (l *CList) Init() *CList {
@@ -231,7 +239,16 @@ func (l *CList) Init() *CList {
 	return l
 }
 
-func New() *CList { return new(CList).Init() }
+// Return CList with MaxLength. CList will panic if it goes beyond MaxLength.
+func New() *CList { return newWithMax(MaxLength) }
+
+// Return CList with given maxLength.
+// Will panic if list exceeds given maxLength.
+func newWithMax(maxLength int) *CList {
+	l := new(CList)
+	l.maxLen = maxLength
+	return l.Init()
+}
 
 func (l *CList) Len() int {
 	l.mtx.RLock()
@@ -295,6 +312,7 @@ func (l *CList) WaitChan() <-chan struct{} {
 	return l.waitCh
 }
 
+// Panics if list grows beyond its max length.
 func (l *CList) PushBack(v interface{}) *CElement {
 	l.mtx.Lock()
 
@@ -315,6 +333,9 @@ func (l *CList) PushBack(v interface{}) *CElement {
 		l.wg.Done()
 		close(l.waitCh)
 	}
+	if l.len >= l.maxLen {
+		panic(fmt.Sprintf("clist: maximum length list reached %d", l.maxLen))
+	}
 	l.len++
 
 	// Modify the tail
diff --git a/libs/clist/clist_test.go b/libs/clist/clist_test.go
index f6653d22..4ded6177 100644
--- a/libs/clist/clist_test.go
+++ b/libs/clist/clist_test.go
@@ -7,9 +7,22 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
+func TestPanicOnMaxLength(t *testing.T) {
+	maxLength := 1000
+
+	l := newWithMax(maxLength)
+	for i := 0; i < maxLength; i++ {
+		l.PushBack(1)
+	}
+	assert.Panics(t, func() {
+		l.PushBack(1)
+	})
+}
+
 func TestSmall(t *testing.T) {
 	l := New()
 	el1 := l.PushBack(1)