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

Commit

Permalink
Check slice length when unmarshaling
Browse files Browse the repository at this point in the history
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!).
  • Loading branch information
spenczar committed Oct 5, 2017
1 parent 51c81e0 commit 91fdfce
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 0 deletions.
17 changes: 17 additions & 0 deletions fuzz.go
Original file line number Diff line number Diff line change
@@ -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

}
28 changes: 28 additions & 0 deletions fuzz_test.go
Original file line number Diff line number Diff line change
@@ -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(")))
}
6 changes: 6 additions & 0 deletions serde.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 91fdfce

Please sign in to comment.