Check slice length when unmarshaling

spenczar · spenczar · commit 91fdfcefc423 · 2017-10-05T10:02:37.000-04:00
With the help of github.com/dvyukov/go-fuzz, two potential panics were
discovered when unmarshaling a TDigest. Since we are setting a slice's
len from the bytes we unpack, we need to validate the len.

If that len is negative, we get a panic immediately when we try to
make the slice.

If that len is excessively huge, we can crash because allocating a
gigantic pile of memory takes excessively long (and we probably don't
want to allocate that much memory anyway!).
diff --git a/fuzz.go b/fuzz.go
@@ -0,0 +1,17 @@
+// +build gofuzz
+
+package tdigest
+
+func Fuzz(data []byte) int {
+	v := new(TDigest)
+	err := v.UnmarshalBinary(data)
+	if err != nil {
+		return 0
+	}
+	_, err = v.MarshalBinary()
+	if err != nil {
+		panic(err)
+	}
+	return 1
+
+}
diff --git a/fuzz_test.go b/fuzz_test.go
@@ -0,0 +1,28 @@
+package tdigest
+
+import "testing"
+
+func TestFuzzPanicRegressions(t *testing.T) {
+	// This test contains a list of byte sequences discovered by
+	// github.com/dvyukov/go-fuzz which, at one time, caused tdigest to panic. The
+	// test just makes sure that they no longer cause a panic.
+	testcase := func(crasher []byte) func(*testing.T) {
+		return func(*testing.T) {
+			v := new(TDigest)
+			err := v.UnmarshalBinary(crasher)
+			if err != nil {
+				return
+			}
+			_, err = v.MarshalBinary()
+			if err != nil {
+				panic(err)
+			}
+		}
+	}
+	t.Run("fuzz1", testcase([]byte(
+		"\x01\x00\x00\x000000000000000000"+
+			"00000000000\xfc")))
+	t.Run("fuzz2", testcase([]byte(
+		"\x01\x00\x00\x00\xdbF_\xbd\xdbF\x00\xbd\xe0\xdfʫ7172"+
+			"73747576777879(")))
+}
diff --git a/serde.go b/serde.go
@@ -53,6 +53,12 @@ func unmarshalBinary(d *TDigest, p []byte) error {
 	if r.err != nil {
 		return r.err
 	}
+	if n < 0 {
+		return fmt.Errorf("invalid n, cannot be negative: %v", n)
+	}
+	if n > 1<<20 {
+		return fmt.Errorf("invalid n, cannot be greater than 2^20: %v", n)
+	}
 	d.centroids = make([]*centroid, int(n))
 	for i := 0; i < int(n); i++ {
 		c := new(centroid)
