From bbaf72ead9849a8bff58d0edec44973bfc28c3d2 Mon Sep 17 00:00:00 2001
From: Richard Patel <me@terorie.dev>
Date: Sat, 20 Aug 2022 09:58:45 +0200
Subject: [PATCH] decoder: add slice len bounds checks

---
 decoder.go             |  6 ++++++
 decoder_bin.go         | 13 ++++++++-----
 decoder_borsh.go       |  4 ++++
 decoder_compact-u16.go |  5 +++++
 decoder_test.go        | 11 ++++++++++-
 variant.go             |  8 ++++----
 6 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/decoder.go b/decoder.go
index 6109539..d341e12 100644
--- a/decoder.go
+++ b/decoder.go
@@ -245,12 +245,18 @@ func (dec *Decoder) ReadLength() (length int, err error) {
 		if err != nil {
 			return 0, err
 		}
+		if val > 0x7FFF_FFFF {
+			return 0, io.ErrUnexpectedEOF
+		}
 		length = int(val)
 	case EncodingBorsh:
 		val, err := dec.ReadUint32(LE)
 		if err != nil {
 			return 0, err
 		}
+		if val > 0x7FFF_FFFF {
+			return 0, io.ErrUnexpectedEOF
+		}
 		length = int(val)
 	case EncodingCompactU16:
 		val, err := DecodeCompactU16LengthFromByteReader(dec)
diff --git a/decoder_bin.go b/decoder_bin.go
index 0c5e056..d4b12d9 100644
--- a/decoder_bin.go
+++ b/decoder_bin.go
@@ -20,6 +20,7 @@ package bin
 import (
 	"encoding/binary"
 	"fmt"
+	"io"
 	"reflect"
 
 	"go.uber.org/zap"
@@ -169,18 +170,21 @@ func (dec *Decoder) decodeBin(rv reflect.Value, opt *option) (err error) {
 		if opt.hasSizeOfSlice() {
 			l = opt.getSizeOfSlice()
 		} else {
-			// TODO: what type is length? Is it really Uvarint64?
-			length, err := dec.ReadUvarint64()
+			length, err := dec.ReadLength()
 			if err != nil {
 				return err
 			}
-			l = int(length)
+			l = length
 		}
 
 		if traceEnabled {
 			zlog.Debug("reading slice", zap.Int("len", l), typeField("type", rv))
 		}
 
+		if l > dec.Remaining() {
+			return io.ErrUnexpectedEOF
+		}
+
 		rv.Set(reflect.MakeSlice(rt, l, l))
 		for i := 0; i < l; i++ {
 			if err = dec.decodeBin(rv.Index(i), nil); err != nil {
@@ -194,8 +198,7 @@ func (dec *Decoder) decodeBin(rv reflect.Value, opt *option) (err error) {
 		}
 
 	case reflect.Map:
-		// TODO: what type is length? Is it really Uvarint64?
-		l, err := dec.ReadUvarint64()
+		l, err := dec.ReadLength()
 		if err != nil {
 			return err
 		}
diff --git a/decoder_borsh.go b/decoder_borsh.go
index fa2c4b1..4da42b5 100644
--- a/decoder_borsh.go
+++ b/decoder_borsh.go
@@ -20,6 +20,7 @@ package bin
 import (
 	"errors"
 	"fmt"
+	"io"
 	"reflect"
 
 	"go.uber.org/zap"
@@ -200,6 +201,9 @@ func (dec *Decoder) decodeBorsh(rv reflect.Value, opt *option) (err error) {
 			// Empty slices are left nil
 			return
 		}
+		if l > dec.Remaining() {
+			return io.ErrUnexpectedEOF
+		}
 
 		rv.Set(reflect.MakeSlice(rt, l, l))
 		for i := 0; i < l; i++ {
diff --git a/decoder_compact-u16.go b/decoder_compact-u16.go
index a4f7ec9..a7d4519 100644
--- a/decoder_compact-u16.go
+++ b/decoder_compact-u16.go
@@ -19,6 +19,7 @@ package bin
 
 import (
 	"fmt"
+	"io"
 	"reflect"
 
 	"go.uber.org/zap"
@@ -179,6 +180,10 @@ func (dec *Decoder) decodeCompactU16(rv reflect.Value, opt *option) (err error)
 			zlog.Debug("reading slice", zap.Int("len", l), typeField("type", rv))
 		}
 
+		if l > dec.Remaining() {
+			return io.ErrUnexpectedEOF
+		}
+
 		rv.Set(reflect.MakeSlice(rt, l, l))
 		for i := 0; i < l; i++ {
 			if err = dec.decodeCompactU16(rv.Index(i), nil); err != nil {
diff --git a/decoder_test.go b/decoder_test.go
index bf9c1f4..a148e98 100644
--- a/decoder_test.go
+++ b/decoder_test.go
@@ -599,7 +599,16 @@ func TestDecoder_Slice_Err(t *testing.T) {
 
 	decoder = NewBinDecoder(buf)
 	err = decoder.Decode(&s)
-	assert.EqualError(t, err, "decode: uint64 required [8] bytes, remaining [0]")
+	assert.EqualError(t, err, "unexpected EOF")
+}
+
+func TestDecoder_Slice_InvalidLen(t *testing.T) {
+	buf := []byte{0xd7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}
+
+	decoder := NewBinDecoder(buf)
+	var s []string
+	err := decoder.Decode(&s)
+	assert.EqualError(t, err, "unexpected EOF")
 }
 
 func TestDecoder_Int64(t *testing.T) {
diff --git a/variant.go b/variant.go
index ef62a25..7cbe40b 100644
--- a/variant.go
+++ b/variant.go
@@ -147,10 +147,10 @@ var NoTypeIDDefaultID = TypeIDFromUint8(0)
 
 // NewVariantDefinition creates a variant definition based on the *ordered* provided types.
 //
-// - For anchor instructions, it's the name that defines the binary variant value.
-// - For all other types, it's the ordering that defines the binary variant value just like in native `nodeos` C++
-//   and in Smart Contract via the `std::variant` type. It's important to pass the entries
-//   in the right order!
+//   - For anchor instructions, it's the name that defines the binary variant value.
+//   - For all other types, it's the ordering that defines the binary variant value just like in native `nodeos` C++
+//     and in Smart Contract via the `std::variant` type. It's important to pass the entries
+//     in the right order!
 //
 // This variant definition can now be passed to functions of `BaseVariant` to implement
 // marshal/unmarshaling functionalities for binary & JSON.