Merge pull request #11 from jmank88/fuzz

github.com/dvyukov/go-fuzz

decode.go

@@ -8,25 +8,34 @@ import (
 	"github.com/pkg/errors"
 )
 
-// An Decoder provides methods for decoding UBJSON data types.
+// MaxCollectionAlloc is the default maximum collection capacity allocation.
+// Can be overridden via Decoder.MaxCollectionAlloc.
+const MaxCollectionAlloc = 1 << 24
+
+// Decoder provides methods for decoding UBJSON data types.
 type Decoder struct {
 	reader
-	// The readValType function is called to get the next value's type
-	// marker. Normally it reads the next marker, but strongly typed
+	// readValType is called to get the next value's type marker.
+	// Normally just reads the next marker, but strongly typed
 	// containers will do an internal check and also manage counters.
 	readValType func() (Marker, error)
+	// Limits the capacity of allocated collections and returns errors rather
+	// than risking waste or panicking on unreasonable/malicious input.
+	// Example: "[[][$][T][#][l][999999999999999999]".
+	// New Decoders default to package const MaxCollectionAlloc.
+	MaxCollectionAlloc int
 }
 
 // NewDecoder returns a new Decoder.
 func NewDecoder(r io.Reader) *Decoder {
-	d := &Decoder{reader: newBinaryReader(r)}
+	d := &Decoder{reader: newBinaryReader(r), MaxCollectionAlloc: MaxCollectionAlloc}
 	d.readValType = d.readMarker
 	return d
 }
 
 // NewBlockDecoder returns a new block-notation Decoder.
 func NewBlockDecoder(r io.Reader) *Decoder {
-	d := &Decoder{reader: newBlockReader(r)}
+	d := &Decoder{reader: newBlockReader(r), MaxCollectionAlloc: MaxCollectionAlloc}
 	d.readValType = d.readMarker
 	return d
 }
@@ -175,7 +184,7 @@ func (d *Decoder) DecodeHighPrecNumber() (string, error) {
 	var v string
 	return v, d.decodeValue(HighPrecNumMarker, func(*Decoder) error {
 		var err error
-		v, err = d.readString()
+		v, err = d.readString(d.MaxCollectionAlloc)
 		return err
 	})
 }
@@ -195,7 +204,7 @@ func (d *Decoder) DecodeString() (string, error) {
 	var v string
 	return v, d.decodeValue(StringMarker, func(*Decoder) error {
 		var err error
-		v, err = d.readString()
+		v, err = d.readString(d.MaxCollectionAlloc)
 		return err
 	})
 }
@@ -237,10 +246,10 @@ func (d *Decoder) decodeInterface() (interface{}, error) {
 		return d.readFloat64()
 
 	case StringMarker:
-		return d.readString()
+		return d.readString(d.MaxCollectionAlloc)
 
 	case HighPrecNumMarker:
-		s, err := d.readString()
+		s, err := d.readString(d.MaxCollectionAlloc)
 		return HighPrecNumber(s), err
 
 	case CharMarker:
@@ -284,9 +293,14 @@ func (d *Decoder) Object() (*ObjectDecoder, error) {
 	if err != nil {
 		return nil, err
 	}
-	o := &ObjectDecoder{ValType: m, Len: l}
-
-	o.Decoder.reader = d.reader
+	o := &ObjectDecoder{
+		Decoder: Decoder{
+			reader:             d.reader,
+			MaxCollectionAlloc: d.MaxCollectionAlloc,
+		},
+		ValType: m,
+		Len:     l,
+	}
 	o.Decoder.readValType = o.readValType
 
 	return o, nil
@@ -311,9 +325,14 @@ func (d *Decoder) Array() (*ArrayDecoder, error) {
 		return nil, err
 	}
 
-	a := &ArrayDecoder{ElemType: m, Len: l}
-
-	a.Decoder.reader = d.reader
+	a := &ArrayDecoder{
+		Decoder: Decoder{
+			reader:             d.reader,
+			MaxCollectionAlloc: d.MaxCollectionAlloc,
+		},
+		ElemType: m,
+		Len:      l,
+	}
 	a.Decoder.readValType = a.readElemType
 
 	return a, nil
@@ -359,7 +378,7 @@ func (o *ObjectDecoder) DecodeKey() (string, error) {
 	if o.count%2 == 0 {
 		return "", errors.New("unable to decode key: expected value")
 	}
-	return o.readString()
+	return o.readString(o.MaxCollectionAlloc)
 }
 
 // NextEntry returns true if more entries are expected, or false if the end has
@@ -629,6 +648,8 @@ func arrayToSlice(slicePtr reflect.Value) func(*ArrayDecoder) error {
 				}
 				sliceValue = reflect.Append(sliceValue, elemPtr.Elem())
 			}
+		} else if ad.Len > ad.MaxCollectionAlloc {
+			return errors.Errorf("collection exceeds max allocation limit of %d: %d", ad.MaxCollectionAlloc, ad.Len)
 		} else {
 			sliceValue.Set(reflect.MakeSlice(sliceValue.Type(), ad.Len, ad.Len))
 
@@ -681,6 +702,9 @@ func fieldByName(structValue reflect.Value, k string) reflect.Value {
 
 func objectIntoMap(mapPtr reflect.Value) func(*ObjectDecoder) error {
 	return func(o *ObjectDecoder) error {
+		if o.Len > o.MaxCollectionAlloc {
+			return errors.Errorf("collection exceeds max allocation limit of %d: %d", o.MaxCollectionAlloc, o.Len)
+		}
 		mapValue := mapPtr.Elem()
 		mapValue.Set(makeMap(mapValue.Type(), o.Len))
 		elemType := mapValue.Type().Elem()
@@ -706,9 +730,11 @@ func objectIntoMap(mapPtr reflect.Value) func(*ObjectDecoder) error {
 // objectAsInterface reads an object and returns a map[string]T where T is
 // either interface{} or a stricter type if the object is strongly typed.
 func objectAsInterface(o *ObjectDecoder) (interface{}, error) {
+	if o.Len > o.MaxCollectionAlloc {
+		return nil, errors.Errorf("collection exceeds max allocation limit of %d: %d", o.MaxCollectionAlloc, o.Len)
+	}
 	valType := elementTypeFor(o.ValType)
 	mapType := reflect.MapOf(stringType, valType)
-
 	mapValue := makeMap(mapType, o.Len)
 	for o.NextEntry() {
 		k, err := o.DecodeKey()
@@ -741,6 +767,8 @@ func arrayAsInterface(a *ArrayDecoder) (interface{}, error) {
 			}
 			sliceValue = reflect.Append(sliceValue, elemPtr.Elem())
 		}
+	} else if a.Len > a.MaxCollectionAlloc {
+		return "", errors.Errorf("collection exceeds max allocation limit of %d: %d", a.MaxCollectionAlloc, a.Len)
 	} else {
 		sliceValue = reflect.MakeSlice(sliceType, a.Len, a.Len)
 

decode_test.go

@@ -2,6 +2,7 @@ package ubjson
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -67,3 +68,70 @@ func TestUnmarshalDiscardUnknownFields(t *testing.T) {
 		t.Errorf("\nexpected: %T %v \nbut got:  %T %v", exp, exp, got, got)
 	}
 }
+
+func TestFuzzUnmarshalCrashers(t *testing.T) {
+	for _, data := range []string{
+		"[$F#i\x8a\x98b\x82ϟ6/\x9b\"\xe4\x88\xe8\xf0\xe0\f1A",
+		"{l[ca[l1ca[ll[ca[l[ca[lcca[l[caP",
+		"{$F#l2y2pY_9A__9y8vqOcl8Vxz9_Lu_2_wl8o4EMgH7T_3yDa8aS05Q17_YMAQHnwZfbccI_5c4",
+		"[#lL00U",
+		"{#l0000",
+		"[l[ca[lca[l[[#l[ca[l",
+		"{#ll[/\xfa\x00\x00\xfa\x80\xff\xff\xff\x01U",
+		"[[#U\x01[#U\x01[#U\x01[#U\x01[#U\x01[#lca[l",
+		"SlS\xfa\xb2S\xaad\xf3#",
+		"Sl\u007f\x00\x00\x00",
+		"SlSl\xaad\xf3#\xaad\xf3#",
+		"Slintterer",
+		"[#L00000000",
+		"[[{I\xda0",
+		"[{I\xda0",
+		"[[[{I\xda0",
+		"{I\x00\x00{I\x800",
+		"Slen\x03\xe8r",
+		"SI\x800",
+		"{I\xe90",
+		"{I\xfa0",
+		"Sl\xff000",
+	} {
+		data := data
+		t.Run(data, func(t *testing.T) {
+			var i interface{}
+			_ = Unmarshal([]byte(data), &i)
+		})
+	}
+}
+
+func TestFuzzUnmarshalBlockCrashers(t *testing.T) {
+	for _, data := range []string{
+		"[[][$][F][#][I][-7][I]4]",
+		"[[][$][T][#][l][1020846876]",
+		"[]",
+		"[[][[][[][[][H][]",
+		"[[][C][]",
+		"[[][$][]",
+		"[[][S][]",
+		"[[][d][7][d][3][1d][7][d][3][d][7]",
+		"[[][[][[][S][]",
+		"[C][]",
+		"[[][[][S][]",
+		"[S][]",
+	} {
+		data := data
+		t.Run(data, func(t *testing.T) {
+			var i interface{}
+			_ = UnmarshalBlock([]byte(data), &i)
+		})
+	}
+}
+
+func TestDecoder_maxCollectionAlloc(t *testing.T) {
+	d := NewBlockDecoder(strings.NewReader(
+		"[[][$][U][#][U][2][76][127]",
+	))
+	d.MaxCollectionAlloc = 1
+	_, err := d.decodeInterface()
+	if err == nil {
+		t.Error("expected error")
+	}
+}

encode.go

@@ -8,11 +8,12 @@ import (
 	"github.com/pkg/errors"
 )
 
-// An Encoder provides methods for encoding UBJSON data types.
+// Encoder provides methods for encoding UBJSON data types.
 type Encoder struct {
 	writer
-	// Function to write value type markers. Normally writeMarker, but
-	// overridden by containers to do validation and optimization.
+	// writeValType is called to write value type markers.
+	// Normally (*Encoder).writeMarker, but overridden by
+	// containers to do validation and optimization.
 	writeValType func(Marker) error
 }
 

fuzz.go

@@ -0,0 +1,25 @@
+// +build gofuzz
+
+package ubjson
+
+// example run: go-fuzz -bin=fuzz-bin.zip -workdir=testdata/bin
+
+//go:generate go-fuzz-build -func FuzzUnmarshal -o fuzz-bin.zip github.com/jmank88/ubjson
+
+func FuzzUnmarshal(data []byte) int {
+	var i interface{}
+	if Unmarshal(data, &i) != nil {
+		return 0
+	}
+	return 1
+}
+
+//go:generate go-fuzz-build -func FuzzUnmarshalBlock -o fuzz-block.zip github.com/jmank88/ubjson
+
+func FuzzUnmarshalBlock(data []byte) int {
+	var i interface{}
+	if UnmarshalBlock(data, &i) != nil {
+		return 0
+	}
+	return 1
+}

reader.go

@@ -27,7 +27,7 @@ type reader interface {
 	readFloat32() (float32, error)
 	readFloat64() (float64, error)
 
-	readString() (string, error)
+	readString(max int) (string, error)
 	readChar() (byte, error)
 }
 
@@ -84,6 +84,9 @@ func readContainer(r reader) (Marker, int, error) {
 		if err != nil {
 			return 0, 0, err
 		}
+		if l < 0 {
+			return 0, 0, errors.Errorf("illegal negative container length: %d", l)
+		}
 		return m, l, nil
 
 	case countMarker:
@@ -94,6 +97,9 @@ func readContainer(r reader) (Marker, int, error) {
 		if err != nil {
 			return 0, 0, err
 		}
+		if l < 0 {
+			return 0, 0, errors.Errorf("illegal negative container length: %d", l)
+		}
 		return 0, l, nil
 
 	default:
@@ -200,11 +206,17 @@ func (r *binaryReader) readFloat64() (float64, error) {
 	return math.Float64frombits(binary.BigEndian.Uint64(b)), err
 }
 
-func (r *binaryReader) readString() (string, error) {
+func (r *binaryReader) readString(max int) (string, error) {
 	l, err := readInt(r)
 	if err != nil {
 		return "", errors.Wrap(err, "failed to read string length prefix")
 	}
+	if l < 1 {
+		return "", errors.Errorf("illegal string length prefix: %d", l)
+	}
+	if l > max {
+		return "", errors.Errorf("string length prefix exceeds max allocation limit of %d: %d", max, l)
+	}
 	b := make([]byte, l)
 	n, err := r.Read(b)
 	if err != nil {
@@ -280,8 +292,8 @@ func (r *blockReader) readMarker() (Marker, error) {
 	if err != nil {
 		return 0, err
 	}
-	if len(s) > 1 {
-		return 0, fmt.Errorf("expected single byte marker, but found: %s", s)
+	if len(s) != 1 {
+		return 0, fmt.Errorf("expected single byte marker, but found: %q", s)
 	}
 	return Marker(s[0]), nil
 }
@@ -359,11 +371,17 @@ func (r *blockReader) readFloat64() (float64, error) {
 	return f, err
 }
 
-func (r *blockReader) readString() (string, error) {
+func (r *blockReader) readString(max int) (string, error) {
 	l, err := readInt(r)
 	if err != nil {
 		return "", errors.Wrap(err, "failed to read string length prefix")
 	}
+	if l < 1 {
+		return "", errors.Errorf("illegal string length prefix: %d", l)
+	}
+	if l > max {
+		return "", errors.Errorf("string length prefix exceeds max allocation limit of %d: %d", max, l)
+	}
 	s, err := r.nextBlock()
 	if err != nil {
 		return "", errors.Wrap(err, "failed to read string block")
@@ -379,8 +397,8 @@ func (r *blockReader) readChar() (byte, error) {
 	if err != nil {
 		return 0, err
 	}
-	if len(s) > 1 {
-		return 0, fmt.Errorf("expected single byte Char, but found: %s", s)
+	if len(s) != 1 {
+		return 0, fmt.Errorf("expected single byte Char, but found: %q", s)
 	}
 	b := s[0]
 	if b > 127 {

testdata/bin/corpus/00460010b9a115f19b7de78f9c3db36e2b11b2b4-12

@@ -0,0 +1 @@
+[]

testdata/bin/corpus/009f860f16fa672f23a19ec74797920d38f9250f-1

@@ -0,0 +1 @@
+[D][000000]

testdata/bin/corpus/00a63e8896c117bacce840b3a8d6d500bef430ee-3

@@ -0,0 +1 @@
+d6061

testdata/bin/corpus/00ba3841a241d0501b5346002698f0a52fec5c77-28

@@ -0,0 +1 @@
+[$S#i1i
{i
!i
{i
{i
U

testdata/bin/corpus/00c6433062e2f9441d51808e231c2b4df3bdc8b6-3

@@ -0,0 +1 @@
+[I][-]

testdata/bin/corpus/012942cec86fef25bab37061b42a46e3e2cee711-7

@@ -0,0 +1,5 @@
+[
+
+
+
+]

testdata/bin/corpus/01c62b790d53ec1f08ba150bdd6af9aab85f3887-18

@@ -0,0 +1 @@
+{i
U{i
U{i
U

testdata/bin/corpus/02aa629c8b16cd17a44f3a0efec2feed43937642-3

@@ -0,0 +1 @@
+S

testdata/bin/corpus/042dc4512fa3d391c5170cf3aa61e6a638f84342-2

@@ -0,0 +1 @@
+i