Skip to content

Commit a64c05b

Browse files
aaronbuchwaldaaronbuchwald
committed
Migrate ABI unpack fix
1 parent 7df91f0 commit a64c05b

File tree

3 files changed

+223
-23
lines changed

3 files changed

+223
-23
lines changed

‎accounts/abi/error_handling.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,15 @@ import (
3333
)
3434

3535
var (
36-
errBadBool = errors.New("abi: improperly encoded boolean value")
36+
errBadBool = errors.New("abi: improperly encoded boolean value")
37+
errBadUint8 = errors.New("abi: improperly encoded uint8 value")
38+
errBadUint16 = errors.New("abi: improperly encoded uint16 value")
39+
errBadUint32 = errors.New("abi: improperly encoded uint32 value")
40+
errBadUint64 = errors.New("abi: improperly encoded uint64 value")
41+
errBadInt8 = errors.New("abi: improperly encoded int8 value")
42+
errBadInt16 = errors.New("abi: improperly encoded int16 value")
43+
errBadInt32 = errors.New("abi: improperly encoded int32 value")
44+
errBadInt64 = errors.New("abi: improperly encoded int64 value")
3745
)
3846

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

‎accounts/abi/unpack.go‎

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ package abi
2929
import (
3030
"encoding/binary"
3131
"fmt"
32+
"math"
3233
"math/big"
3334
"reflect"
3435

@@ -43,43 +44,72 @@ var (
4344
)
4445

4546
// ReadInteger reads the integer based on its kind and returns the appropriate value.
46-
func ReadInteger(typ Type, b []byte) interface{} {
47+
func ReadInteger(typ Type, b []byte) (interface{}, error) {
48+
ret := new(big.Int).SetBytes(b)
49+
4750
if typ.T == UintTy {
51+
u64, isu64 := ret.Uint64(), ret.IsUint64()
4852
switch typ.Size {
4953
case 8:
50-
return b[len(b)-1]
54+
if !isu64 || u64 > math.MaxUint8 {
55+
return nil, errBadUint8
56+
}
57+
return byte(u64), nil
5158
case 16:
52-
return binary.BigEndian.Uint16(b[len(b)-2:])
59+
if !isu64 || u64 > math.MaxUint16 {
60+
return nil, errBadUint16
61+
}
62+
return uint16(u64), nil
5363
case 32:
54-
return binary.BigEndian.Uint32(b[len(b)-4:])
64+
if !isu64 || u64 > math.MaxUint32 {
65+
return nil, errBadUint32
66+
}
67+
return uint32(u64), nil
5568
case 64:
56-
return binary.BigEndian.Uint64(b[len(b)-8:])
69+
if !isu64 {
70+
return nil, errBadUint64
71+
}
72+
return u64, nil
5773
default:
5874
// the only case left for unsigned integer is uint256.
59-
return new(big.Int).SetBytes(b)
75+
return ret, nil
6076
}
6177
}
78+
79+
// big.SetBytes can't tell if a number is negative or positive in itself.
80+
// On EVM, if the returned number > max int256, it is negative.
81+
// A number is > max int256 if the bit at position 255 is set.
82+
if ret.Bit(255) == 1 {
83+
ret.Add(MaxUint256, new(big.Int).Neg(ret))
84+
ret.Add(ret, common.Big1)
85+
ret.Neg(ret)
86+
}
87+
i64, isi64 := ret.Int64(), ret.IsInt64()
6288
switch typ.Size {
6389
case 8:
64-
return int8(b[len(b)-1])
90+
if !isi64 || i64 < math.MinInt8 || i64 > math.MaxInt8 {
91+
return nil, errBadInt8
92+
}
93+
return int8(i64), nil
6594
case 16:
66-
return int16(binary.BigEndian.Uint16(b[len(b)-2:]))
95+
if !isi64 || i64 < math.MinInt16 || i64 > math.MaxInt16 {
96+
return nil, errBadInt16
97+
}
98+
return int16(i64), nil
6799
case 32:
68-
return int32(binary.BigEndian.Uint32(b[len(b)-4:]))
100+
if !isi64 || i64 < math.MinInt32 || i64 > math.MaxInt32 {
101+
return nil, errBadInt32
102+
}
103+
return int32(i64), nil
69104
case 64:
70-
return int64(binary.BigEndian.Uint64(b[len(b)-8:]))
105+
if !isi64 {
106+
return nil, errBadInt64
107+
}
108+
return i64, nil
71109
default:
72110
// the only case left for integer is int256
73-
// big.SetBytes can't tell if a number is negative or positive in itself.
74-
// On EVM, if the returned number > max int256, it is negative.
75-
// A number is > max int256 if the bit at position 255 is set.
76-
ret := new(big.Int).SetBytes(b)
77-
if ret.Bit(255) == 1 {
78-
ret.Add(MaxUint256, new(big.Int).Neg(ret))
79-
ret.Add(ret, common.Big1)
80-
ret.Neg(ret)
81-
}
82-
return ret
111+
112+
return ret, nil
83113
}
84114
}
85115

@@ -133,7 +163,7 @@ func forEachUnpack(t Type, output []byte, start, size int) (interface{}, error)
133163
return nil, fmt.Errorf("cannot marshal input to array, size is negative (%d)", size)
134164
}
135165
if start+32*size > len(output) {
136-
return nil, fmt.Errorf("abi: cannot marshal in to go array: offset %d would go over slice boundary (len=%d)", len(output), start+32*size)
166+
return nil, fmt.Errorf("abi: cannot marshal into go array: offset %d would go over slice boundary (len=%d)", len(output), start+32*size)
137167
}
138168

139169
// this value will become our slice or our array, depending on the type
@@ -244,7 +274,7 @@ func toGoType(index int, t Type, output []byte) (interface{}, error) {
244274
case StringTy: // variable arrays are written at the end of the return bytes
245275
return string(output[begin : begin+length]), nil
246276
case IntTy, UintTy:
247-
return ReadInteger(t, returnOutput), nil
277+
return ReadInteger(t, returnOutput)
248278
case BoolTy:
249279
return readBool(returnOutput)
250280
case AddressTy:

‎accounts/abi/unpack_test.go‎

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

0 commit comments

Comments
 (0)