Skip to content
This repository was archived by the owner on Jan 20, 2023. It is now read-only.

Commit 91fdfce

Browse files
committed
Check slice length when unmarshaling
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!).
1 parent 51c81e0 commit 91fdfce

File tree

3 files changed

+51
-0
lines changed

3 files changed

+51
-0
lines changed

fuzz.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// +build gofuzz
2+
3+
package tdigest
4+
5+
func Fuzz(data []byte) int {
6+
v := new(TDigest)
7+
err := v.UnmarshalBinary(data)
8+
if err != nil {
9+
return 0
10+
}
11+
_, err = v.MarshalBinary()
12+
if err != nil {
13+
panic(err)
14+
}
15+
return 1
16+
17+
}

fuzz_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package tdigest
2+
3+
import "testing"
4+
5+
func TestFuzzPanicRegressions(t *testing.T) {
6+
// This test contains a list of byte sequences discovered by
7+
// github.com/dvyukov/go-fuzz which, at one time, caused tdigest to panic. The
8+
// test just makes sure that they no longer cause a panic.
9+
testcase := func(crasher []byte) func(*testing.T) {
10+
return func(*testing.T) {
11+
v := new(TDigest)
12+
err := v.UnmarshalBinary(crasher)
13+
if err != nil {
14+
return
15+
}
16+
_, err = v.MarshalBinary()
17+
if err != nil {
18+
panic(err)
19+
}
20+
}
21+
}
22+
t.Run("fuzz1", testcase([]byte(
23+
"\x01\x00\x00\x000000000000000000"+
24+
"00000000000\xfc")))
25+
t.Run("fuzz2", testcase([]byte(
26+
"\x01\x00\x00\x00\xdbF_\xbd\xdbF\x00\xbd\xe0\xdfʫ7172"+
27+
"73747576777879(")))
28+
}

serde.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ func unmarshalBinary(d *TDigest, p []byte) error {
5353
if r.err != nil {
5454
return r.err
5555
}
56+
if n < 0 {
57+
return fmt.Errorf("invalid n, cannot be negative: %v", n)
58+
}
59+
if n > 1<<20 {
60+
return fmt.Errorf("invalid n, cannot be greater than 2^20: %v", n)
61+
}
5662
d.centroids = make([]*centroid, int(n))
5763
for i := 0; i < int(n); i++ {
5864
c := new(centroid)

0 commit comments

Comments
 (0)