decoder: sanity-check the length of slices/maps before allocating them (

#19)

xdr3/decode.go

@@ -25,14 +25,36 @@ import (
 	"time"
 )
 
-const maxInt32 = int(^uint32(0) >> 1)
+const maxInt32 = math.MaxInt32
 
 var errMaxSlice = "data exceeds max slice limit"
 var errIODecode = "%s while decoding %d bytes"
 
 // DecodeDefaultMaxDepth is the default maximum decoding depth
 const DecodeDefaultMaxDepth = 200
 
+// DecodeOptions configures how Decoding is done.
+type DecodeOptions struct {
+	// MaxDepth is the maximum decoding depth (i.e. maximum nesting of data structures).
+	// It prevents infinite recursions in cyclic datastructures and determines the maximum callstack growth.
+	// If set to 0, DecodeDefaultMaxDepth will be used.
+	MaxDepth uint
+
+	// MaxInputLen sets the maximum input size. It is used by the decoder to sanity-check
+	// allocation sizes and avoid heap explosions from doctored inputs.
+	//
+	// If set to 0, the decoder will try to figure out the input size by checking whether
+	// the provided io.Reader implements Len() (e.g. strings.Reader, bytes.Reader and bytes.Buffer do).
+	// Otherwise, no sanity checks will be done.
+	MaxInputLen int
+}
+
+// DefaultDecodeOptions are the default decoding options.
+var DefaultDecodeOptions = DecodeOptions{
+	MaxDepth:    DecodeDefaultMaxDepth,
+	MaxInputLen: 0,
+}
+
 /*
 Unmarshal parses XDR-encoded data into the value pointed to by v reading from
 reader r and returning the total number of bytes read.  An addressable pointer
@@ -66,7 +88,7 @@ by v and performs a mapping of underlying XDR types to Go types as follows:
 Notes and Limitations:
 
   - Automatic unmarshalling of variable and fixed-length arrays of uint8s
-    requires a special struct tag `xdropaque:"false"` since byte slices
+    requires a special struct tag xdropaque:"false" since byte slices
     and byte arrays are assumed to be opaque data and byte is a Go alias
     for uint8 thus indistinguishable under reflection
   - Cyclic data structures are not supported and will result in ErrMaxDecodingDepth errors
@@ -78,23 +100,81 @@ potential issues are unsupported Go types, attempting to decode a value which is
 too large to fit into a specified Go type, and exceeding max slice limitations.
 */
 func Unmarshal(r io.Reader, v interface{}) (int, error) {
-	d := Decoder{r: r}
+	d := NewDecoder(r)
+	return d.Decode(v)
+}
+
+// UnmarshalWithOptions works like Unmarshal but accepts decoding options.
+func UnmarshalWithOptions(r io.Reader, v interface{}, options DecodeOptions) (int, error) {
+	d := NewDecoderWithOptions(r, options)
 	return d.Decode(v)
 }
 
+type lenLeft interface {
+	Len() int
+}
+
 // A Decoder wraps an io.Reader that is expected to provide an XDR-encoded byte
 // stream and provides several exposed methods to manually decode various XDR
 // primitives without relying on reflection.  The NewDecoder function can be
 // used to get a new Decoder directly.
 //
 // Typically, Unmarshal should be used instead of manual decoding.  A Decoder
-// is exposed so it is possible to perform manual decoding should it be
+// is exposed, so it is possible to perform manual decoding should it be
 // necessary in complex scenarios where automatic reflection-based decoding
 // won't work.
 type Decoder struct {
 	// used to minimize heap allocations during decoding
 	scratchBuf [8]byte
 	r          io.Reader
+	l          lenLeft
+	maxDepth   uint
+}
+
+// readerLenWrapper wraps a reader an initial length and provides a Len() method indicating
+// how much input is left
+type readerLenWrapper struct {
+	inner      io.Reader
+	readCount  int
+	initialLen int
+}
+
+func (l *readerLenWrapper) Len() int {
+	return l.initialLen - l.readCount
+}
+
+func (l *readerLenWrapper) Read(p []byte) (int, error) {
+	n, err := l.inner.Read(p)
+	if n > 0 {
+		l.readCount += n
+	}
+	return n, err
+}
+
+// NewDecoder returns a Decoder that can be used to manually decode XDR data
+// from a provided reader. Typically, Unmarshal should be used instead of
+// manually creating a Decoder.
+func NewDecoder(r io.Reader) *Decoder {
+	return NewDecoderWithOptions(r, DefaultDecodeOptions)
+}
+
+// NewDecoderWithOptions works like NewDecoder but allows supplying decoding options.
+func NewDecoderWithOptions(r io.Reader, options DecodeOptions) *Decoder {
+	maxDepth := options.MaxDepth
+	if maxDepth < 1 {
+		maxDepth = DecodeDefaultMaxDepth
+	}
+	if l, ok := r.(lenLeft); ok {
+		return &Decoder{r: r, l: l, maxDepth: maxDepth}
+	}
+	if options.MaxInputLen > 0 {
+		rlw := &readerLenWrapper{
+			inner:      r,
+			initialLen: options.MaxInputLen,
+		}
+		return &Decoder{r: rlw, l: rlw, maxDepth: maxDepth}
+	}
+	return &Decoder{r: r, l: nil, maxDepth: options.MaxDepth}
 }
 
 // DecodeInt treats the next 4 bytes as an XDR encoded integer and returns the
@@ -383,6 +463,7 @@ func (d *Decoder) DecodeOpaque(maxSize int) ([]byte, int, error) {
 		return nil, n, err
 	}
 
+	maxSize = d.mergeInputLenAndMaxSize(maxSize)
 	if maxSize == 0 {
 		maxSize = maxInt32
 	}
@@ -422,6 +503,7 @@ func (d *Decoder) DecodeString(maxSize int) (string, int, error) {
 		return "", n, err
 	}
 
+	maxSize = d.mergeInputLenAndMaxSize(maxSize)
 	if maxSize == 0 {
 		maxSize = maxInt32
 	}
@@ -477,7 +559,7 @@ func (d *Decoder) decodeFixedArray(v reflect.Value, ignoreOpaque bool, maxDepth
 // elements of the same type as the array represented by the reflection value.
 // The number of elements is obtained by first decoding the unsigned integer
 // element count.  Then each element is decoded into the passed array. The
-// ignoreOpaque flag controls whether or not uint8 (byte) elements should be
+// ignoreOpaque flag controls whether uint8 (byte) elements should be
 // decoded individually or as a variable sequence of opaque data.  It returns
 // the number of bytes actually read.
 //
@@ -495,6 +577,7 @@ func (d *Decoder) decodeArray(v reflect.Value, ignoreOpaque bool, maxSize int, m
 		return n, err
 	}
 
+	maxSize = d.mergeInputLenAndMaxSize(maxSize)
 	if maxSize == 0 {
 		maxSize = maxInt32
 	}
@@ -591,7 +674,8 @@ func (d *Decoder) decodeUnion(v reflect.Value, maxDepth uint) (int, error) {
 
 	vv := v.FieldByName(arm)
 
-	vv.Set(reflect.New(vv.Type().Elem()))
+	vvet := vv.Type().Elem()
+	vv.Set(reflect.New(vvet))
 
 	field, ok := v.Type().FieldByName(arm)
 	if !ok {
@@ -621,8 +705,8 @@ func (d *Decoder) decodeUnion(v reflect.Value, maxDepth uint) (int, error) {
 
 // decodeStruct treats the next bytes as a series of XDR encoded elements
 // of the same type as the exported fields of the struct represented by the
-// passed reflection value.  Pointers are automatically indirected and
-// allocated as necessary.  It returns the  the number of bytes actually read.
+// passed reflection value. Pointers are automatically indirected and
+// allocated as necessary. It returns the number of bytes actually read.
 //
 // An UnmarshalError is returned if any issues are encountered while decoding
 // the elements.
@@ -724,6 +808,11 @@ func (d *Decoder) decodeMap(v reflect.Value, maxDepth uint) (int, error) {
 	if err != nil {
 		return n, err
 	}
+	if left, ok := d.InputLen(); ok {
+		if uint(left) < uint(dataLen) {
+			return n, unmarshalError("decodeMap", ErrOverflow, errMaxSlice, dataLen, nil)
+		}
+	}
 
 	// Allocate storage for the underlying map if needed.
 	vt := v.Type()
@@ -756,7 +845,7 @@ func (d *Decoder) decodeMap(v reflect.Value, maxDepth uint) (int, error) {
 // decodeInterface examines the interface represented by the passed reflection
 // value to detect whether it is an interface that can be decoded into and
 // if it is, extracts the underlying value to pass back into the decode function
-// for decoding according to its type.  It returns the  the number of bytes
+// for decoding according to its type. It returns the number of bytes
 // actually read.
 //
 // An UnmarshalError is returned if any issues are encountered while decoding
@@ -786,6 +875,15 @@ func (d *Decoder) decodeInterface(v reflect.Value, maxDepth uint) (int, error) {
 	return d.decode(ve, 0, maxDepth)
 }
 
+func (d *Decoder) mergeInputLenAndMaxSize(maxSize int) int {
+	if left, ok := d.InputLen(); ok {
+		if maxSize == 0 || left < maxSize {
+			return left
+		}
+	}
+	return maxSize
+}
+
 // decode is the main workhorse for unmarshalling via reflection.  It uses
 // the passed reflection value to choose the XDR primitives to decode from
 // the encapsulated reader.  It is a recursive function,
@@ -993,7 +1091,7 @@ func setPtrToNil(v *reflect.Value) error {
 	return nil
 }
 
-func allocPtrIfNil(v *reflect.Value) error {
+func (d *Decoder) allocPtrIfNil(v *reflect.Value) error {
 	if v.Kind() != reflect.Ptr {
 		msg := fmt.Sprintf("value is not a pointer: '%v'",
 			v.Type().String())
@@ -1010,7 +1108,8 @@ func allocPtrIfNil(v *reflect.Value) error {
 		return err
 	}
 	if isNil {
-		v.Set(reflect.New(v.Type().Elem()))
+		vet := v.Type().Elem()
+		v.Set(reflect.New(vet))
 	}
 	return nil
 }
@@ -1030,7 +1129,7 @@ func (d *Decoder) decodePtr(v reflect.Value, maxDepth uint) (int, error) {
 		return n, err
 	}
 
-	if err = allocPtrIfNil(&v); err != nil {
+	if err = d.allocPtrIfNil(&v); err != nil {
 		return n, err
 	}
 
@@ -1042,7 +1141,7 @@ func (d *Decoder) decodePtr(v reflect.Value, maxDepth uint) (int, error) {
 // otherwise returns the passed value.
 func (d *Decoder) indirectIfPtr(v reflect.Value) (reflect.Value, error) {
 	if v.Kind() == reflect.Ptr {
-		err := allocPtrIfNil(&v)
+		err := d.allocPtrIfNil(&v)
 		return v.Elem(), err
 	}
 	return v, nil
@@ -1053,11 +1152,6 @@ func (d *Decoder) indirectIfPtr(v reflect.Value) (reflect.Value, error) {
 // data instead of a user-supplied reader. See the Unmarhsal documentation for
 // specifics. Decode(v) is equivalent to DecodeWithMaxDepth(v, DecodeDefaultMaxDepth)
 func (d *Decoder) Decode(v interface{}) (int, error) {
-	return d.DecodeWithMaxDepth(v, DecodeDefaultMaxDepth)
-}
-
-// DecodeWithMaxDepth behaves like Decode, except an explicit maximum decoding depth is used
-func (d *Decoder) DecodeWithMaxDepth(v interface{}, maxDepth uint) (int, error) {
 	if v == nil {
 		msg := "can't unmarshal to nil interface"
 		return 0, unmarshalError("Unmarshal", ErrNilInterface, msg, nil,
@@ -1078,12 +1172,13 @@ func (d *Decoder) DecodeWithMaxDepth(v interface{}, maxDepth uint) (int, error)
 		return 0, err
 	}
 
-	return d.decode(vv.Elem(), 0, maxDepth)
+	return d.decode(vv.Elem(), 0, d.maxDepth)
 }
 
-// NewDecoder returns a Decoder that can be used to manually decode XDR data
-// from a provided reader.  Typically, Unmarshal should be used instead of
-// manually creating a Decoder.
-func NewDecoder(r io.Reader) *Decoder {
-	return &Decoder{r: r}
+// InputLen returns the size left to read from the decoder's input if available
+func (d *Decoder) InputLen() (int, bool) {
+	if d.l == nil {
+		return 0, false
+	}
+	return d.l.Len(), true
 }

xdr3/decode_test.go

@@ -18,6 +18,7 @@ package xdr_test
 
 import (
 	"bytes"
+	"encoding/base64"
 	"fmt"
 	"math"
 	"reflect"
@@ -361,7 +362,7 @@ func TestUnmarshal(t *testing.T) {
 		// element no extra bytes, 1 map element not enough bytes for
 		// key, 1 map element not enough bytes for value.
 		{[]byte{0x00, 0x00, 0x00}, map[string]uint32{}, 3, &UnmarshalError{ErrorCode: ErrIO}},
-		{[]byte{0x00, 0x00, 0x00, 0x01}, map[string]uint32{}, 4, &UnmarshalError{ErrorCode: ErrIO}},
+		{[]byte{0x00, 0x00, 0x00, 0x01}, map[string]uint32{}, 4, &UnmarshalError{ErrorCode: ErrOverflow}},
 		{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00}, map[string]uint32{}, 7, &UnmarshalError{ErrorCode: ErrIO}},
 		{[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x6D, 0x61, 0x70, 0x31},
 			map[string]uint32{}, 12, &UnmarshalError{ErrorCode: ErrIO}},
@@ -1143,15 +1144,51 @@ func TestDecodeMaxDepth(t *testing.T) {
 	}
 
 	bufCopy := buf
-	decoder := NewDecoder(&bufCopy)
+	decoder := NewDecoderWithOptions(&bufCopy, DecodeOptions{MaxDepth: 3})
 	var s structWithPointer
-	_, err = decoder.DecodeWithMaxDepth(&s, 3)
+	_, err = decoder.Decode(&s)
 	if err != nil {
 		t.Error("unexpected error")
 	}
 
 	bufCopy = buf
-	decoder = NewDecoder(&bufCopy)
-	_, err = decoder.DecodeWithMaxDepth(&s, 2)
+	decoder = NewDecoderWithOptions(&bufCopy, DecodeOptions{MaxDepth: 2})
+	_, err = decoder.Decode(&s)
 	assertError(t, "", err, &UnmarshalError{ErrorCode: ErrMaxDecodingDepth})
 }
+
+func TestDecodeMaxAllocationCheck_ImplicitLenReader(t *testing.T) {
+	var buf bytes.Buffer
+	_, err := Marshal(&buf, "thisstringis23charslong")
+	if err != nil {
+		t.Error("unexpected error")
+	}
+
+	// Reduce the buffer size so that the length of the buffer
+	// is shorter than the encoded XDR  length
+	buf.Truncate(buf.Len() - 4)
+
+	decoder := NewDecoder(&buf)
+	var s string
+	_, err = decoder.Decode(&s)
+	assertError(t, "", err, &UnmarshalError{ErrorCode: ErrOverflow})
+}
+
+func TestDecodeMaxAllocationCheck_ExplicitLenReader(t *testing.T) {
+	var buf bytes.Buffer
+	encoder := base64.NewEncoder(base64.StdEncoding, &buf)
+	_, err := Marshal(encoder, "thisstringis23charslong")
+	if err != nil {
+		t.Error("unexpected error")
+	}
+
+	xdrLen := base64.StdEncoding.DecodedLen(buf.Len())
+	// Reduce the buffer size so that the length of the buffer
+	// is shorter than the encoded XDR length
+	reducedLen := xdrLen - 4
+
+	decoder := NewDecoderWithOptions(&buf, DecodeOptions{MaxInputLen: reducedLen})
+	var s string
+	_, err = decoder.Decode(&s)
+	assertError(t, "", err, &UnmarshalError{ErrorCode: ErrOverflow})
+}

xdr3/error.go

@@ -43,7 +43,8 @@ const (
 	ErrNotSettable
 
 	// ErrOverflow indicates that the data in question is too large to fit
-	// into the corresponding Go or XDR data type.  For example, an integer
+	// into the corresponding Go or XDR data type or that allocating it exceeds the
+	// maximum allocation size limit.  For example, an integer
 	// decoded from XDR that is too large to fit into a target type of int8,
 	// or opaque data that exceeds the max length of a Go slice.
 	ErrOverflow

xdr3/internal_test.go

@@ -37,7 +37,7 @@ func TstEncode(w io.Writer) func(v reflect.Value) (int, error) {
 
 // TstDecode creates a new Decoder for the passed reader and returns the
 // internal decode function on the Decoder.
-func TstDecode(r io.Reader) func(v reflect.Value, maxSize int, maxDepth uint) (int, error) {
+func TstDecode(r io.Reader) func(v reflect.Value, maxLen int, maxDepth uint) (int, error) {
 	dec := NewDecoder(r)
 	return dec.decode
 }