From cefc0fa00fb4fa289104cbeb7cc809e07bbf35b8 Mon Sep 17 00:00:00 2001 From: zhiqiangxu <652732310@qq.com> Date: Tue, 7 Feb 2023 21:32:27 +0800 Subject: [PATCH] accounts/abi: fix integer encoding/decoding (#26568) This PR fixes this abi encoder/decoder to be more stringent. --- accounts/abi/error_handling.go | 10 +- accounts/abi/unpack.go | 72 ++++++++++----- accounts/abi/unpack_test.go | 162 +++++++++++++++++++++++++++++++++ 3 files changed, 222 insertions(+), 22 deletions(-) diff --git a/accounts/abi/error_handling.go b/accounts/abi/error_handling.go index 7add70729..c106e9ac4 100644 --- a/accounts/abi/error_handling.go +++ b/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 diff --git a/accounts/abi/unpack.go b/accounts/abi/unpack.go index b6ca0a038..1e778a6ff 100644 --- a/accounts/abi/unpack.go +++ b/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: diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index 363e0cd59..6dd2db0d5 100644 --- a/accounts/abi/unpack_test.go +++ b/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]) + } + } +}