Bug decode abi bytes (#235)

* Add test

* Fix abi decode dynamic length items

* Add Decode panic test

Author: ferranbt

abi/abi_test.go

@@ -247,49 +247,49 @@ func TestAbi_HumanReadable(t *testing.T) {
 			Inputs: MustNewType("tuple(string symbol, string name)"),
 		},
 		Methods: map[string]*Method{
-			"transferFrom": &Method{
+			"transferFrom": {
 				Name:    "transferFrom",
 				Inputs:  MustNewType("tuple(address from, address to, uint256 value)"),
 				Outputs: MustNewType("tuple()"),
 			},
-			"balanceOf": &Method{
+			"balanceOf": {
 				Name:    "balanceOf",
 				Inputs:  MustNewType("tuple(address owner)"),
 				Outputs: MustNewType("tuple(uint256 balance)"),
 			},
-			"balanceOf0": &Method{
+			"balanceOf0": {
 				Name:    "balanceOf",
 				Inputs:  MustNewType("tuple()"),
 				Outputs: MustNewType("tuple()"),
 			},
-			"addPerson": &Method{
+			"addPerson": {
 				Name:    "addPerson",
 				Inputs:  MustNewType("tuple(tuple(string name, uint16 age) person)"),
 				Outputs: MustNewType("tuple()"),
 			},
-			"addPeople": &Method{
+			"addPeople": {
 				Name:    "addPeople",
 				Inputs:  MustNewType("tuple(tuple(string name, uint16 age)[] person)"),
 				Outputs: MustNewType("tuple()"),
 			},
-			"getPerson": &Method{
+			"getPerson": {
 				Name:    "getPerson",
 				Inputs:  MustNewType("tuple(uint256 id)"),
 				Outputs: MustNewType("tuple(tuple(string name, uint16 age))"),
 			},
 		},
 		Events: map[string]*Event{
-			"Transfer": &Event{
+			"Transfer": {
 				Name:   "Transfer",
 				Inputs: MustNewType("tuple(address indexed from, address indexed to, address value)"),
 			},
-			"PersonAdded": &Event{
+			"PersonAdded": {
 				Name:   "PersonAdded",
 				Inputs: MustNewType("tuple(uint256 indexed id, tuple(string name, uint16 age) person)"),
 			},
 		},
 		Errors: map[string]*Error{
-			"InsufficientBalance": &Error{
+			"InsufficientBalance": {
 				Name:   "InsufficientBalance",
 				Inputs: MustNewType("tuple(address owner, uint256 balance)"),
 			},

abi/decode.go

@@ -296,7 +296,10 @@ func readLength(data []byte) (int, error) {
 		return 0, fmt.Errorf("length larger than int64: %v", lengthBig.Int64())
 	}
 	length := int(lengthBig.Uint64())
-	if length > len(data) {
+
+	// if we trim the length in the data there should be enough
+	// bytes to cover the length
+	if length > len(data)-32 {
 		return 0, fmt.Errorf("length insufficient %v require %v", len(data), length)
 	}
 	return length, nil

abi/decode_test.go

@@ -1,8 +1,20 @@
 package abi
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
 
 func TestDecode_BytesBound(t *testing.T) {
 	typ := MustNewType("tuple(string)")
 	decodeTuple(typ, nil) // it should not panic
 }
+
+func TestDecode_DynamicLengthOutOfBounds(t *testing.T) {
+	input := []byte("00000000000000000000000000000000\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00 00000000000000000000000000")
+	typ := MustNewType("tuple(bytes32, bytes, bytes)")
+
+	_, err := Decode(typ, input)
+	require.Error(t, err)
+}

abi/encoding_test.go

@@ -621,10 +621,35 @@ func TestRandomEncoding(t *testing.T) {
 			if err := testEncodeDecode(t, server, tt, input); err != nil {
 				t.Fatal(err)
 			}
+
+			if err := testDecodePanic(tt, input); err != nil {
+				t.Fatal(err)
+			}
 		})
 	}
 }
 
+func testDecodePanic(tt *Type, input interface{}) error {
+	// test that the encoded input and random permutattions of the response do not cause
+	// panics on Decode function
+	res1, err := Encode(input, tt)
+	if err != nil {
+		return err
+	}
+
+	buf := make([]byte, len(res1))
+
+	// change each bit of the input with 1
+	for i := 0; i < len(res1); i++ {
+		copy(buf, res1)
+		buf[i] = 0xff
+
+		Decode(tt, buf)
+	}
+
+	return nil
+}
+
 func testTypeWithContract(t *testing.T, server *testutil.TestServer, typ *Type) error {
 	g := &generateContractImpl{}
 	source := g.run(typ)