From c3379da2f57869ff5c060c69ece1637d7df68185 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Fri, 9 May 2025 13:32:35 +0800 Subject: [PATCH 1/5] accounts/abi: return an error if attempting to pack a negative big.Int into a uint val --- accounts/abi/pack.go | 13 +++++++++++++ accounts/abi/pack_test.go | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/accounts/abi/pack.go b/accounts/abi/pack.go index beef1fa37fa7..01761c37c4aa 100644 --- a/accounts/abi/pack.go +++ b/accounts/abi/pack.go @@ -33,11 +33,24 @@ func packBytesSlice(bytes []byte, l int) []byte { return append(len, common.RightPadBytes(bytes, (l+31)/32*32)...) } +func validateNum(t Type, reflectValue reflect.Value) error { + if t.T == UintTy && reflectValue.Kind() == reflect.Ptr { + val := new(big.Int).Set(reflectValue.Interface().(*big.Int)) + if val.Sign() == -1 { + return fmt.Errorf("cannot pack negative big.Int value to a uint type") + } + } + return nil +} + // packElement packs the given reflect value according to the abi specification in // t. func packElement(t Type, reflectValue reflect.Value) ([]byte, error) { switch t.T { case IntTy, UintTy: + if err := validateNum(t, reflectValue); err != nil { + return nil, err + } return packNum(reflectValue), nil case StringTy: return packBytesSlice([]byte(reflectValue.String()), reflectValue.Len()), nil diff --git a/accounts/abi/pack_test.go b/accounts/abi/pack_test.go index cda31b6204d7..72f372d40df0 100644 --- a/accounts/abi/pack_test.go +++ b/accounts/abi/pack_test.go @@ -177,6 +177,13 @@ func TestMethodPack(t *testing.T) { if !bytes.Equal(packed, sig) { t.Errorf("expected %x got %x", sig, packed) } + + _, err = abi.Pack("send", big.NewInt(-1)) + if err == nil { + t.Fatal("expected error when trying to pack negative big.Int into uint256 value") + } + + // TODO: examine what happens if we supply a negative non-big int to a method expect smaller uint val } func TestPackNumber(t *testing.T) { From fce0d3e846b543daa4ebfb767650c02610a40828 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Fri, 9 May 2025 14:51:33 +0800 Subject: [PATCH 2/5] add func docs --- accounts/abi/pack.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/accounts/abi/pack.go b/accounts/abi/pack.go index 01761c37c4aa..35590df1dd5e 100644 --- a/accounts/abi/pack.go +++ b/accounts/abi/pack.go @@ -33,6 +33,8 @@ func packBytesSlice(bytes []byte, l int) []byte { return append(len, common.RightPadBytes(bytes, (l+31)/32*32)...) } +// validateNum returns an error if the underlying type of t is uint and the +// value of reflectValue is a negative big.Int func validateNum(t Type, reflectValue reflect.Value) error { if t.T == UintTy && reflectValue.Kind() == reflect.Ptr { val := new(big.Int).Set(reflectValue.Interface().(*big.Int)) From bf8ea8ece04314a5fcf57f5775bb9642690cda99 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Fri, 9 May 2025 14:55:38 +0800 Subject: [PATCH 3/5] comment tweaks --- accounts/abi/pack_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/accounts/abi/pack_test.go b/accounts/abi/pack_test.go index 72f372d40df0..0de1c10cf586 100644 --- a/accounts/abi/pack_test.go +++ b/accounts/abi/pack_test.go @@ -178,12 +178,11 @@ func TestMethodPack(t *testing.T) { t.Errorf("expected %x got %x", sig, packed) } + // test that we can't pack a negative value for a parameter that is specified as a uint _, err = abi.Pack("send", big.NewInt(-1)) if err == nil { t.Fatal("expected error when trying to pack negative big.Int into uint256 value") } - - // TODO: examine what happens if we supply a negative non-big int to a method expect smaller uint val } func TestPackNumber(t *testing.T) { From 9574d543d0e5faf39d2f0e7331f6e2b5460fbedc Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Fri, 9 May 2025 16:00:16 +0800 Subject: [PATCH 4/5] fix test case --- accounts/abi/error_handling.go | 19 +++++----- accounts/abi/pack.go | 2 +- accounts/abi/unpack_test.go | 66 ++++++++++++++++++---------------- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/accounts/abi/error_handling.go b/accounts/abi/error_handling.go index c106e9ac4321..9ef96c571b4c 100644 --- a/accounts/abi/error_handling.go +++ b/accounts/abi/error_handling.go @@ -23,15 +23,16 @@ import ( ) var ( - 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") + 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") + errInvalidSign = errors.New("abi: negatively-signed value cannot be packed into uint parameter") ) // formatSliceString formats the reflection kind with the given slice size diff --git a/accounts/abi/pack.go b/accounts/abi/pack.go index 35590df1dd5e..78c2b87cbf4e 100644 --- a/accounts/abi/pack.go +++ b/accounts/abi/pack.go @@ -39,7 +39,7 @@ func validateNum(t Type, reflectValue reflect.Value) error { if t.T == UintTy && reflectValue.Kind() == reflect.Ptr { val := new(big.Int).Set(reflectValue.Interface().(*big.Int)) if val.Sign() == -1 { - return fmt.Errorf("cannot pack negative big.Int value to a uint type") + return errInvalidSign } } return nil diff --git a/accounts/abi/unpack_test.go b/accounts/abi/unpack_test.go index 74db298b3f37..90713c03ca22 100644 --- a/accounts/abi/unpack_test.go +++ b/accounts/abi/unpack_test.go @@ -1014,128 +1014,134 @@ func TestPackAndUnpackIncompatibleNumber(t *testing.T) { cases := []struct { decodeType string inputValue *big.Int - err error + unpackErr error + packErr error expectValue interface{} }{ { decodeType: "uint8", inputValue: big.NewInt(math.MaxUint8 + 1), - err: errBadUint8, + unpackErr: errBadUint8, }, { decodeType: "uint8", inputValue: big.NewInt(math.MaxUint8), - err: nil, + unpackErr: nil, expectValue: uint8(math.MaxUint8), }, { decodeType: "uint16", inputValue: big.NewInt(math.MaxUint16 + 1), - err: errBadUint16, + unpackErr: errBadUint16, }, { decodeType: "uint16", inputValue: big.NewInt(math.MaxUint16), - err: nil, + unpackErr: nil, expectValue: uint16(math.MaxUint16), }, { decodeType: "uint32", inputValue: big.NewInt(math.MaxUint32 + 1), - err: errBadUint32, + unpackErr: errBadUint32, }, { decodeType: "uint32", inputValue: big.NewInt(math.MaxUint32), - err: nil, + unpackErr: nil, expectValue: uint32(math.MaxUint32), }, { decodeType: "uint64", inputValue: maxU64Plus1, - err: errBadUint64, + unpackErr: errBadUint64, }, { decodeType: "uint64", inputValue: maxU64, - err: nil, + unpackErr: nil, expectValue: uint64(math.MaxUint64), }, { decodeType: "uint256", inputValue: maxU64Plus1, - err: nil, + unpackErr: nil, expectValue: maxU64Plus1, }, { decodeType: "int8", inputValue: big.NewInt(math.MaxInt8 + 1), - err: errBadInt8, + unpackErr: errBadInt8, }, { - decodeType: "int8", inputValue: big.NewInt(math.MinInt8 - 1), - err: errBadInt8, + packErr: errInvalidSign, }, { decodeType: "int8", inputValue: big.NewInt(math.MaxInt8), - err: nil, + unpackErr: nil, expectValue: int8(math.MaxInt8), }, { decodeType: "int16", inputValue: big.NewInt(math.MaxInt16 + 1), - err: errBadInt16, + unpackErr: errBadInt16, }, { - decodeType: "int16", inputValue: big.NewInt(math.MinInt16 - 1), - err: errBadInt16, + packErr: errInvalidSign, }, { decodeType: "int16", inputValue: big.NewInt(math.MaxInt16), - err: nil, + unpackErr: nil, expectValue: int16(math.MaxInt16), }, { decodeType: "int32", inputValue: big.NewInt(math.MaxInt32 + 1), - err: errBadInt32, + unpackErr: errBadInt32, }, { - decodeType: "int32", inputValue: big.NewInt(math.MinInt32 - 1), - err: errBadInt32, + packErr: errInvalidSign, }, { decodeType: "int32", inputValue: big.NewInt(math.MaxInt32), - err: nil, + unpackErr: nil, expectValue: int32(math.MaxInt32), }, { decodeType: "int64", inputValue: new(big.Int).Add(big.NewInt(math.MaxInt64), big.NewInt(1)), - err: errBadInt64, + unpackErr: errBadInt64, }, { - decodeType: "int64", inputValue: new(big.Int).Sub(big.NewInt(math.MinInt64), big.NewInt(1)), - err: errBadInt64, + packErr: errInvalidSign, }, { decodeType: "int64", inputValue: big.NewInt(math.MaxInt64), - err: nil, + unpackErr: nil, expectValue: int64(math.MaxInt64), }, } for i, testCase := range cases { packed, err := encodeABI.Pack(testCase.inputValue) - if err != nil { - panic(err) + if testCase.packErr != nil { + if err == nil { + t.Fatalf("expected packing of testcase input value to fail") + } + if err != testCase.packErr { + t.Fatalf("expected error '%v', got '%v'", testCase.packErr, err) + } + continue + } + if err != nil && err != testCase.packErr { + panic(fmt.Errorf("unexpected error packing test-case input: %v", err)) } ty, err := NewType(testCase.decodeType, "", nil) if err != nil { @@ -1145,8 +1151,8 @@ func TestPackAndUnpackIncompatibleNumber(t *testing.T) { {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 != testCase.unpackErr { + t.Fatalf("Expected error %v, actual error %v. case %d", testCase.unpackErr, err, i) } if err != nil { continue From 566afae9bf7db8615343400dfd080b006ffaa158 Mon Sep 17 00:00:00 2001 From: MariusVanDerWijden Date: Wed, 4 Jun 2025 14:42:59 +0200 Subject: [PATCH 5/5] accounts/abi: style changes --- accounts/abi/pack.go | 24 +++++++++--------------- accounts/abi/pack_test.go | 3 +-- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/accounts/abi/pack.go b/accounts/abi/pack.go index 78c2b87cbf4e..a4c73922d4fb 100644 --- a/accounts/abi/pack.go +++ b/accounts/abi/pack.go @@ -33,27 +33,21 @@ func packBytesSlice(bytes []byte, l int) []byte { return append(len, common.RightPadBytes(bytes, (l+31)/32*32)...) } -// validateNum returns an error if the underlying type of t is uint and the -// value of reflectValue is a negative big.Int -func validateNum(t Type, reflectValue reflect.Value) error { - if t.T == UintTy && reflectValue.Kind() == reflect.Ptr { - val := new(big.Int).Set(reflectValue.Interface().(*big.Int)) - if val.Sign() == -1 { - return errInvalidSign - } - } - return nil -} - // packElement packs the given reflect value according to the abi specification in // t. func packElement(t Type, reflectValue reflect.Value) ([]byte, error) { switch t.T { - case IntTy, UintTy: - if err := validateNum(t, reflectValue); err != nil { - return nil, err + case UintTy: + // make sure to not pack a negative value into a uint type. + if reflectValue.Kind() == reflect.Ptr { + val := new(big.Int).Set(reflectValue.Interface().(*big.Int)) + if val.Sign() == -1 { + return nil, errInvalidSign + } } return packNum(reflectValue), nil + case IntTy: + return packNum(reflectValue), nil case StringTy: return packBytesSlice([]byte(reflectValue.String()), reflectValue.Len()), nil case AddressTy: diff --git a/accounts/abi/pack_test.go b/accounts/abi/pack_test.go index 0de1c10cf586..d1e3fbbf6946 100644 --- a/accounts/abi/pack_test.go +++ b/accounts/abi/pack_test.go @@ -179,8 +179,7 @@ func TestMethodPack(t *testing.T) { } // test that we can't pack a negative value for a parameter that is specified as a uint - _, err = abi.Pack("send", big.NewInt(-1)) - if err == nil { + if _, err := abi.Pack("send", big.NewInt(-1)); err == nil { t.Fatal("expected error when trying to pack negative big.Int into uint256 value") } }