accounts/abi: fix integer encoding/decoding (ethereum#26568)

This PR fixes this abi encoder/decoder to be more stringent.

accounts/abi/error_handling.go

@@ -23,7 +23,15 @@ import (
 )
 
 var (
-	errBadBool = errors.New("abi: improperly encoded boolean value")
+	errBadBool   = errors.New("abi: improperly encoded boolean value")
+	errBadUint8  = errors.New("abi: improperly encoded uint8 value")
+	errBadUint16 = errors.New("abi: improperly encoded uint16 value")
+	errBadUint32 = errors.New("abi: improperly encoded uint32 value")
+	errBadUint64 = errors.New("abi: improperly encoded uint64 value")
+	errBadInt8   = errors.New("abi: improperly encoded int8 value")
+	errBadInt16  = errors.New("abi: improperly encoded int16 value")
+	errBadInt32  = errors.New("abi: improperly encoded int32 value")
+	errBadInt64  = errors.New("abi: improperly encoded int64 value")
 )
 
 // formatSliceString formats the reflection kind with the given slice size

accounts/abi/unpack.go

@@ -19,6 +19,7 @@ package abi
 import (
 	"encoding/binary"
 	"fmt"
+	"math"
 	"math/big"
 	"reflect"
 
@@ -33,43 +34,72 @@ var (
 )
 
 // ReadInteger reads the integer based on its kind and returns the appropriate value.
-func ReadInteger(typ Type, b []byte) interface{} {
+func ReadInteger(typ Type, b []byte) (interface{}, error) {
+	ret := new(big.Int).SetBytes(b)
+
 	if typ.T == UintTy {
+		u64, isu64 := ret.Uint64(), ret.IsUint64()
 		switch typ.Size {
 		case 8:
-			return b[len(b)-1]
+			if !isu64 || u64 > math.MaxUint8 {
+				return nil, errBadUint8
+			}
+			return byte(u64), nil
 		case 16:
-			return binary.BigEndian.Uint16(b[len(b)-2:])
+			if !isu64 || u64 > math.MaxUint16 {
+				return nil, errBadUint16
+			}
+			return uint16(u64), nil
 		case 32:
-			return binary.BigEndian.Uint32(b[len(b)-4:])
+			if !isu64 || u64 > math.MaxUint32 {
+				return nil, errBadUint32
+			}
+			return uint32(u64), nil
 		case 64:
-			return binary.BigEndian.Uint64(b[len(b)-8:])
+			if !isu64 {
+				return nil, errBadUint64
+			}
+			return u64, nil
 		default:
 			// the only case left for unsigned integer is uint256.
-			return new(big.Int).SetBytes(b)
+			return ret, nil
 		}
 	}
+
+	// big.SetBytes can't tell if a number is negative or positive in itself.
+	// On EVM, if the returned number > max int256, it is negative.
+	// A number is > max int256 if the bit at position 255 is set.
+	if ret.Bit(255) == 1 {
+		ret.Add(MaxUint256, new(big.Int).Neg(ret))
+		ret.Add(ret, common.Big1)
+		ret.Neg(ret)
+	}
+	i64, isi64 := ret.Int64(), ret.IsInt64()
 	switch typ.Size {
 	case 8:
-		return int8(b[len(b)-1])
+		if !isi64 || i64 < math.MinInt8 || i64 > math.MaxInt8 {
+			return nil, errBadInt8
+		}
+		return int8(i64), nil
 	case 16:
-		return int16(binary.BigEndian.Uint16(b[len(b)-2:]))
+		if !isi64 || i64 < math.MinInt16 || i64 > math.MaxInt16 {
+			return nil, errBadInt16
+		}
+		return int16(i64), nil
 	case 32:
-		return int32(binary.BigEndian.Uint32(b[len(b)-4:]))
+		if !isi64 || i64 < math.MinInt32 || i64 > math.MaxInt32 {
+			return nil, errBadInt32
+		}
+		return int32(i64), nil
 	case 64:
-		return int64(binary.BigEndian.Uint64(b[len(b)-8:]))
+		if !isi64 {
+			return nil, errBadInt64
+		}
+		return i64, nil
 	default:
 		// the only case left for integer is int256
-		// big.SetBytes can't tell if a number is negative or positive in itself.
-		// On EVM, if the returned number > max int256, it is negative.
-		// A number is > max int256 if the bit at position 255 is set.
-		ret := new(big.Int).SetBytes(b)
-		if ret.Bit(255) == 1 {
-			ret.Add(MaxUint256, new(big.Int).Neg(ret))
-			ret.Add(ret, common.Big1)
-			ret.Neg(ret)
-		}
-		return ret
+
+		return ret, nil
 	}
 }
 
@@ -234,7 +264,7 @@ func toGoType(index int, t Type, output []byte) (interface{}, error) {
 	case StringTy: // variable arrays are written at the end of the return bytes
 		return string(output[begin : begin+length]), nil
 	case IntTy, UintTy:
-		return ReadInteger(t, returnOutput), nil
+		return ReadInteger(t, returnOutput)
 	case BoolTy:
 		return readBool(returnOutput)
 	case AddressTy:

accounts/abi/unpack_test.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
+	"math"
 	"math/big"
 	"reflect"
 	"strconv"
@@ -943,3 +944,164 @@ func TestOOMMaliciousInput(t *testing.T) {
 		}
 	}
 }
+
+func TestPackAndUnpackIncompatibleNumber(t *testing.T) {
+	var encodeABI Arguments
+	uint256Ty, err := NewType("uint256", "", nil)
+	if err != nil {
+		panic(err)
+	}
+	encodeABI = Arguments{
+		{Type: uint256Ty},
+	}
+
+	maxU64, ok := new(big.Int).SetString(strconv.FormatUint(math.MaxUint64, 10), 10)
+	if !ok {
+		panic("bug")
+	}
+	maxU64Plus1 := new(big.Int).Add(maxU64, big.NewInt(1))
+	cases := []struct {
+		decodeType  string
+		inputValue  *big.Int
+		err         error
+		expectValue interface{}
+	}{
+		{
+			decodeType: "uint8",
+			inputValue: big.NewInt(math.MaxUint8 + 1),
+			err:        errBadUint8,
+		},
+		{
+			decodeType:  "uint8",
+			inputValue:  big.NewInt(math.MaxUint8),
+			err:         nil,
+			expectValue: uint8(math.MaxUint8),
+		},
+		{
+			decodeType: "uint16",
+			inputValue: big.NewInt(math.MaxUint16 + 1),
+			err:        errBadUint16,
+		},
+		{
+			decodeType:  "uint16",
+			inputValue:  big.NewInt(math.MaxUint16),
+			err:         nil,
+			expectValue: uint16(math.MaxUint16),
+		},
+		{
+			decodeType: "uint32",
+			inputValue: big.NewInt(math.MaxUint32 + 1),
+			err:        errBadUint32,
+		},
+		{
+			decodeType:  "uint32",
+			inputValue:  big.NewInt(math.MaxUint32),
+			err:         nil,
+			expectValue: uint32(math.MaxUint32),
+		},
+		{
+			decodeType: "uint64",
+			inputValue: maxU64Plus1,
+			err:        errBadUint64,
+		},
+		{
+			decodeType:  "uint64",
+			inputValue:  maxU64,
+			err:         nil,
+			expectValue: uint64(math.MaxUint64),
+		},
+		{
+			decodeType:  "uint256",
+			inputValue:  maxU64Plus1,
+			err:         nil,
+			expectValue: maxU64Plus1,
+		},
+		{
+			decodeType: "int8",
+			inputValue: big.NewInt(math.MaxInt8 + 1),
+			err:        errBadInt8,
+		},
+		{
+			decodeType: "int8",
+			inputValue: big.NewInt(math.MinInt8 - 1),
+			err:        errBadInt8,
+		},
+		{
+			decodeType:  "int8",
+			inputValue:  big.NewInt(math.MaxInt8),
+			err:         nil,
+			expectValue: int8(math.MaxInt8),
+		},
+		{
+			decodeType: "int16",
+			inputValue: big.NewInt(math.MaxInt16 + 1),
+			err:        errBadInt16,
+		},
+		{
+			decodeType: "int16",
+			inputValue: big.NewInt(math.MinInt16 - 1),
+			err:        errBadInt16,
+		},
+		{
+			decodeType:  "int16",
+			inputValue:  big.NewInt(math.MaxInt16),
+			err:         nil,
+			expectValue: int16(math.MaxInt16),
+		},
+		{
+			decodeType: "int32",
+			inputValue: big.NewInt(math.MaxInt32 + 1),
+			err:        errBadInt32,
+		},
+		{
+			decodeType: "int32",
+			inputValue: big.NewInt(math.MinInt32 - 1),
+			err:        errBadInt32,
+		},
+		{
+			decodeType:  "int32",
+			inputValue:  big.NewInt(math.MaxInt32),
+			err:         nil,
+			expectValue: int32(math.MaxInt32),
+		},
+		{
+			decodeType: "int64",
+			inputValue: new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(1)),
+			err:        errBadInt64,
+		},
+		{
+			decodeType: "int64",
+			inputValue: new(big.Int).Sub(big.NewInt(math.MinInt64), big.NewInt(1)),
+			err:        errBadInt64,
+		},
+		{
+			decodeType:  "int64",
+			inputValue:  big.NewInt(math.MaxInt64),
+			err:         nil,
+			expectValue: int64(math.MaxInt64),
+		},
+	}
+	for i, testCase := range cases {
+		packed, err := encodeABI.Pack(testCase.inputValue)
+		if err != nil {
+			panic(err)
+		}
+		ty, err := NewType(testCase.decodeType, "", nil)
+		if err != nil {
+			panic(err)
+		}
+		decodeABI := Arguments{
+			{Type: ty},
+		}
+		decoded, err := decodeABI.Unpack(packed)
+		if err != testCase.err {
+			t.Fatalf("Expected error %v, actual error %v. case %d", testCase.err, err, i)
+		}
+		if err != nil {
+			continue
+		}
+		if !reflect.DeepEqual(decoded[0], testCase.expectValue) {
+			t.Fatalf("Expected value %v, actual value %v", testCase.expectValue, decoded[0])
+		}
+	}
+}