Skip to content

Commit cefc0fa

Browse files
zhiqiangxuzhiqiangxu
authored
accounts/abi: fix integer encoding/decoding (#26568)
This PR fixes this abi encoder/decoder to be more stringent.
1 parent 91cb6f8 commit cefc0fa

File tree

3 files changed

+222
-22
lines changed

3 files changed

+222
-22
lines changed

‎accounts/abi/error_handling.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,15 @@ import (
2323
)
2424

2525
var (
26-
errBadBool = errors.New("abi: improperly encoded boolean value")
26+
errBadBool = errors.New("abi: improperly encoded boolean value")
27+
errBadUint8 = errors.New("abi: improperly encoded uint8 value")
28+
errBadUint16 = errors.New("abi: improperly encoded uint16 value")
29+
errBadUint32 = errors.New("abi: improperly encoded uint32 value")
30+
errBadUint64 = errors.New("abi: improperly encoded uint64 value")
31+
errBadInt8 = errors.New("abi: improperly encoded int8 value")
32+
errBadInt16 = errors.New("abi: improperly encoded int16 value")
33+
errBadInt32 = errors.New("abi: improperly encoded int32 value")
34+
errBadInt64 = errors.New("abi: improperly encoded int64 value")
2735
)
2836

2937
// formatSliceString formats the reflection kind with the given slice size

‎accounts/abi/unpack.go

+51-21
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package abi
1919
import (
2020
"encoding/binary"
2121
"fmt"
22+
"math"
2223
"math/big"
2324
"reflect"
2425

@@ -33,43 +34,72 @@ var (
3334
)
3435

3536
// ReadInteger reads the integer based on its kind and returns the appropriate value.
36-
func ReadInteger(typ Type, b []byte) interface{} {
37+
func ReadInteger(typ Type, b []byte) (interface{}, error) {
38+
ret := new(big.Int).SetBytes(b)
39+
3740
if typ.T == UintTy {
41+
u64, isu64 := ret.Uint64(), ret.IsUint64()
3842
switch typ.Size {
3943
case 8:
40-
return b[len(b)-1]
44+
if !isu64 || u64 > math.MaxUint8 {
45+
return nil, errBadUint8
46+
}
47+
return byte(u64), nil
4148
case 16:
42-
return binary.BigEndian.Uint16(b[len(b)-2:])
49+
if !isu64 || u64 > math.MaxUint16 {
50+
return nil, errBadUint16
51+
}
52+
return uint16(u64), nil
4353
case 32:
44-
return binary.BigEndian.Uint32(b[len(b)-4:])
54+
if !isu64 || u64 > math.MaxUint32 {
55+
return nil, errBadUint32
56+
}
57+
return uint32(u64), nil
4558
case 64:
46-
return binary.BigEndian.Uint64(b[len(b)-8:])
59+
if !isu64 {
60+
return nil, errBadUint64
61+
}
62+
return u64, nil
4763
default:
4864
// the only case left for unsigned integer is uint256.
49-
return new(big.Int).SetBytes(b)
65+
return ret, nil
5066
}
5167
}
68+
69+
// big.SetBytes can't tell if a number is negative or positive in itself.
70+
// On EVM, if the returned number > max int256, it is negative.
71+
// A number is > max int256 if the bit at position 255 is set.
72+
if ret.Bit(255) == 1 {
73+
ret.Add(MaxUint256, new(big.Int).Neg(ret))
74+
ret.Add(ret, common.Big1)
75+
ret.Neg(ret)
76+
}
77+
i64, isi64 := ret.Int64(), ret.IsInt64()
5278
switch typ.Size {
5379
case 8:
54-
return int8(b[len(b)-1])
80+
if !isi64 || i64 < math.MinInt8 || i64 > math.MaxInt8 {
81+
return nil, errBadInt8
82+
}
83+
return int8(i64), nil
5584
case 16:
56-
return int16(binary.BigEndian.Uint16(b[len(b)-2:]))
85+
if !isi64 || i64 < math.MinInt16 || i64 > math.MaxInt16 {
86+
return nil, errBadInt16
87+
}
88+
return int16(i64), nil
5789
case 32:
58-
return int32(binary.BigEndian.Uint32(b[len(b)-4:]))
90+
if !isi64 || i64 < math.MinInt32 || i64 > math.MaxInt32 {
91+
return nil, errBadInt32
92+
}
93+
return int32(i64), nil
5994
case 64:
60-
return int64(binary.BigEndian.Uint64(b[len(b)-8:]))
95+
if !isi64 {
96+
return nil, errBadInt64
97+
}
98+
return i64, nil
6199
default:
62100
// the only case left for integer is int256
63-
// big.SetBytes can't tell if a number is negative or positive in itself.
64-
// On EVM, if the returned number > max int256, it is negative.
65-
// A number is > max int256 if the bit at position 255 is set.
66-
ret := new(big.Int).SetBytes(b)
67-
if ret.Bit(255) == 1 {
68-
ret.Add(MaxUint256, new(big.Int).Neg(ret))
69-
ret.Add(ret, common.Big1)
70-
ret.Neg(ret)
71-
}
72-
return ret
101+
102+
return ret, nil
73103
}
74104
}
75105

@@ -234,7 +264,7 @@ func toGoType(index int, t Type, output []byte) (interface{}, error) {
234264
case StringTy: // variable arrays are written at the end of the return bytes
235265
return string(output[begin : begin+length]), nil
236266
case IntTy, UintTy:
237-
return ReadInteger(t, returnOutput), nil
267+
return ReadInteger(t, returnOutput)
238268
case BoolTy:
239269
return readBool(returnOutput)
240270
case AddressTy:

‎accounts/abi/unpack_test.go

+162
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"bytes"
2121
"encoding/hex"
2222
"fmt"
23+
"math"
2324
"math/big"
2425
"reflect"
2526
"strconv"
@@ -943,3 +944,164 @@ func TestOOMMaliciousInput(t *testing.T) {
943944
}
944945
}
945946
}
947+
948+
func TestPackAndUnpackIncompatibleNumber(t *testing.T) {
949+
var encodeABI Arguments
950+
uint256Ty, err := NewType("uint256", "", nil)
951+
if err != nil {
952+
panic(err)
953+
}
954+
encodeABI = Arguments{
955+
{Type: uint256Ty},
956+
}
957+
958+
maxU64, ok := new(big.Int).SetString(strconv.FormatUint(math.MaxUint64, 10), 10)
959+
if !ok {
960+
panic("bug")
961+
}
962+
maxU64Plus1 := new(big.Int).Add(maxU64, big.NewInt(1))
963+
cases := []struct {
964+
decodeType string
965+
inputValue *big.Int
966+
err error
967+
expectValue interface{}
968+
}{
969+
{
970+
decodeType: "uint8",
971+
inputValue: big.NewInt(math.MaxUint8 + 1),
972+
err: errBadUint8,
973+
},
974+
{
975+
decodeType: "uint8",
976+
inputValue: big.NewInt(math.MaxUint8),
977+
err: nil,
978+
expectValue: uint8(math.MaxUint8),
979+
},
980+
{
981+
decodeType: "uint16",
982+
inputValue: big.NewInt(math.MaxUint16 + 1),
983+
err: errBadUint16,
984+
},
985+
{
986+
decodeType: "uint16",
987+
inputValue: big.NewInt(math.MaxUint16),
988+
err: nil,
989+
expectValue: uint16(math.MaxUint16),
990+
},
991+
{
992+
decodeType: "uint32",
993+
inputValue: big.NewInt(math.MaxUint32 + 1),
994+
err: errBadUint32,
995+
},
996+
{
997+
decodeType: "uint32",
998+
inputValue: big.NewInt(math.MaxUint32),
999+
err: nil,
1000+
expectValue: uint32(math.MaxUint32),
1001+
},
1002+
{
1003+
decodeType: "uint64",
1004+
inputValue: maxU64Plus1,
1005+
err: errBadUint64,
1006+
},
1007+
{
1008+
decodeType: "uint64",
1009+
inputValue: maxU64,
1010+
err: nil,
1011+
expectValue: uint64(math.MaxUint64),
1012+
},
1013+
{
1014+
decodeType: "uint256",
1015+
inputValue: maxU64Plus1,
1016+
err: nil,
1017+
expectValue: maxU64Plus1,
1018+
},
1019+
{
1020+
decodeType: "int8",
1021+
inputValue: big.NewInt(math.MaxInt8 + 1),
1022+
err: errBadInt8,
1023+
},
1024+
{
1025+
decodeType: "int8",
1026+
inputValue: big.NewInt(math.MinInt8 - 1),
1027+
err: errBadInt8,
1028+
},
1029+
{
1030+
decodeType: "int8",
1031+
inputValue: big.NewInt(math.MaxInt8),
1032+
err: nil,
1033+
expectValue: int8(math.MaxInt8),
1034+
},
1035+
{
1036+
decodeType: "int16",
1037+
inputValue: big.NewInt(math.MaxInt16 + 1),
1038+
err: errBadInt16,
1039+
},
1040+
{
1041+
decodeType: "int16",
1042+
inputValue: big.NewInt(math.MinInt16 - 1),
1043+
err: errBadInt16,
1044+
},
1045+
{
1046+
decodeType: "int16",
1047+
inputValue: big.NewInt(math.MaxInt16),
1048+
err: nil,
1049+
expectValue: int16(math.MaxInt16),
1050+
},
1051+
{
1052+
decodeType: "int32",
1053+
inputValue: big.NewInt(math.MaxInt32 + 1),
1054+
err: errBadInt32,
1055+
},
1056+
{
1057+
decodeType: "int32",
1058+
inputValue: big.NewInt(math.MinInt32 - 1),
1059+
err: errBadInt32,
1060+
},
1061+
{
1062+
decodeType: "int32",
1063+
inputValue: big.NewInt(math.MaxInt32),
1064+
err: nil,
1065+
expectValue: int32(math.MaxInt32),
1066+
},
1067+
{
1068+
decodeType: "int64",
1069+
inputValue: new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(1)),
1070+
err: errBadInt64,
1071+
},
1072+
{
1073+
decodeType: "int64",
1074+
inputValue: new(big.Int).Sub(big.NewInt(math.MinInt64), big.NewInt(1)),
1075+
err: errBadInt64,
1076+
},
1077+
{
1078+
decodeType: "int64",
1079+
inputValue: big.NewInt(math.MaxInt64),
1080+
err: nil,
1081+
expectValue: int64(math.MaxInt64),
1082+
},
1083+
}
1084+
for i, testCase := range cases {
1085+
packed, err := encodeABI.Pack(testCase.inputValue)
1086+
if err != nil {
1087+
panic(err)
1088+
}
1089+
ty, err := NewType(testCase.decodeType, "", nil)
1090+
if err != nil {
1091+
panic(err)
1092+
}
1093+
decodeABI := Arguments{
1094+
{Type: ty},
1095+
}
1096+
decoded, err := decodeABI.Unpack(packed)
1097+
if err != testCase.err {
1098+
t.Fatalf("Expected error %v, actual error %v. case %d", testCase.err, err, i)
1099+
}
1100+
if err != nil {
1101+
continue
1102+
}
1103+
if !reflect.DeepEqual(decoded[0], testCase.expectValue) {
1104+
t.Fatalf("Expected value %v, actual value %v", testCase.expectValue, decoded[0])
1105+
}
1106+
}
1107+
}

0 commit comments

Comments
 (0)