From a3cfbb735bb3d0a625c8b3337f96f574a6a21fea Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Tue, 27 Aug 2024 13:47:39 -0400 Subject: [PATCH 1/2] Increase handling of JSON Number large numbers (vs. string) Signed-off-by: Peter Broadhurst --- internal/signermsgs/en_error_messges.go | 3 ++ pkg/abi/inputparsing.go | 65 +++++++++-------------- pkg/ethtypes/hexinteger.go | 29 ++++------- pkg/ethtypes/hexinteger_test.go | 5 +- pkg/ethtypes/hexuint64.go | 25 ++++----- pkg/ethtypes/hexuint64_test.go | 18 ++++++- pkg/ethtypes/integer_parsing.go | 68 +++++++++++++++++++++++++ pkg/ethtypes/integer_parsing_test.go | 46 +++++++++++++++++ 8 files changed, 179 insertions(+), 80 deletions(-) create mode 100644 pkg/ethtypes/integer_parsing.go create mode 100644 pkg/ethtypes/integer_parsing_test.go diff --git a/internal/signermsgs/en_error_messges.go b/internal/signermsgs/en_error_messges.go index 95d832c0..df5751ba 100644 --- a/internal/signermsgs/en_error_messges.go +++ b/internal/signermsgs/en_error_messges.go @@ -104,4 +104,7 @@ var ( MsgInvalidEIP155TransactionV = ffe("FF22085", "Invalid V value from EIP-155 transaction (chainId=%d)") MsgInvalidChainID = ffe("FF22086", "Invalid chainId expected=%d actual=%d") MsgSigningInvalidCompactRSV = ffe("FF22087", "Invalid signature data (compact R,S,V) length=%d (expected=65)") + MsgInvalidNumberString = ffe("FF22088", "Invalid integer string '%s'") + MsgInvalidIntPrecisionLoss = ffe("FF22089", "String %s cannot be converted to integer without losing precision") + MsgInvalidUint64PrecisionLoss = ffe("FF22090", "String %s cannot be converted to a uint64 without losing precision") ) diff --git a/pkg/abi/inputparsing.go b/pkg/abi/inputparsing.go index 296386f1..c347b299 100644 --- a/pkg/abi/inputparsing.go +++ b/pkg/abi/inputparsing.go @@ -19,6 +19,7 @@ package abi import ( "context" "encoding/hex" + "encoding/json" "fmt" "math/big" "reflect" @@ -26,6 +27,7 @@ import ( "github.com/hyperledger/firefly-common/pkg/i18n" "github.com/hyperledger/firefly-signer/internal/signermsgs" + "github.com/hyperledger/firefly-signer/pkg/ethtypes" ) var ( @@ -128,68 +130,49 @@ func getFloat64IfConvertible(v interface{}) (float64, bool) { // with a focus on those generated by the result of an Unmarshal using Go's default // unmarshalling. func getIntegerFromInterface(ctx context.Context, desc string, v interface{}) (*big.Int, error) { - i := new(big.Int) switch vt := v.(type) { + case json.Number: + i, err := ethtypes.BigIntegerFromString(ctx, vt.String()) + if err != nil { + return nil, i18n.WrapError(ctx, err, signermsgs.MsgInvalidIntegerABIInput, vt, v, desc) + } + return i, nil case string: - // We use Go's default '0' base integer parsing, where `0x` means hex, - // no prefix means decimal etc. - i, ok := i.SetString(vt, 0) - if !ok { - f, _, err := big.ParseFloat(vt, 10, 256, big.ToNearestEven) - if err != nil { - return nil, i18n.NewError(ctx, signermsgs.MsgInvalidIntegerABIInput, vt, v, desc) - } - i, accuracy := f.Int(i) - if accuracy != big.Exact { - // If we weren't able to decode without losing precision, return an error - return nil, i18n.NewError(ctx, signermsgs.MsgInvalidIntegerABIInput, vt, v, desc) - } - - return i, nil + i, err := ethtypes.BigIntegerFromString(ctx, vt) + if err != nil { + return nil, i18n.WrapError(ctx, err, signermsgs.MsgInvalidIntegerABIInput, vt, v, desc) } return i, nil case *big.Float: - i, _ := vt.Int(i) + i, _ := vt.Int(nil) return i, nil case *big.Int: return vt, nil case float64: // This is how JSON numbers come in (no distinction between integers/floats) - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case float32: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case int64: - i.SetInt64(vt) - return i, nil + return new(big.Int).SetInt64(vt), nil case int32: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case int16: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case int8: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case int: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case uint64: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetUint64(vt), nil case uint32: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case uint16: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case uint8: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetInt64(int64(vt)), nil case uint: - i.SetInt64(int64(vt)) - return i, nil + return new(big.Int).SetUint64(uint64(vt)), nil default: if str, ok := getStringIfConvertible(v); ok { return getIntegerFromInterface(ctx, desc, str) diff --git a/pkg/ethtypes/hexinteger.go b/pkg/ethtypes/hexinteger.go index 5672d5ca..d18a1e92 100644 --- a/pkg/ethtypes/hexinteger.go +++ b/pkg/ethtypes/hexinteger.go @@ -1,4 +1,4 @@ -// Copyright © 2023 Kaleido, Inc. +// Copyright © 2024 Kaleido, Inc. // // SPDX-License-Identifier: Apache-2.0 // @@ -18,7 +18,6 @@ package ethtypes import ( "context" - "encoding/json" "fmt" "math/big" @@ -37,25 +36,15 @@ func (h HexInteger) MarshalJSON() ([]byte, error) { } func (h *HexInteger) UnmarshalJSON(b []byte) error { - var i interface{} - _ = json.Unmarshal(b, &i) - switch i := i.(type) { - case float64: - *h = HexInteger(*big.NewInt(int64(i))) - return nil - case string: - bi, ok := new(big.Int).SetString(i, 0) - if !ok { - return fmt.Errorf("unable to parse integer: %s", i) - } - if bi.Sign() < 0 { - return fmt.Errorf("negative values are not supported: %s", i) - } - *h = HexInteger(*bi) - return nil - default: - return fmt.Errorf("unable to parse integer from type %T", i) + bi, err := UnmarshalBigInt(b) + if err != nil { + return err + } + if bi.Sign() < 0 { + return fmt.Errorf("negative values are not supported: %s", b) } + *h = HexInteger(*bi) + return nil } func (h *HexInteger) BigInt() *big.Int { diff --git a/pkg/ethtypes/hexinteger_test.go b/pkg/ethtypes/hexinteger_test.go index 097f976a..f473381e 100644 --- a/pkg/ethtypes/hexinteger_test.go +++ b/pkg/ethtypes/hexinteger_test.go @@ -73,7 +73,10 @@ func TestHexIntegerMissingBytes(t *testing.T) { }` err := json.Unmarshal([]byte(testData), &testStruct) - assert.Regexp(t, "unable to parse integer", err) + assert.Regexp(t, "FF22088", err) + + err = testStruct.I1.UnmarshalJSON([]byte(`{!badJSON`)) + assert.Regexp(t, "invalid", err) } func TestHexIntegerBadType(t *testing.T) { diff --git a/pkg/ethtypes/hexuint64.go b/pkg/ethtypes/hexuint64.go index 4c7f757d..bbfb02ac 100644 --- a/pkg/ethtypes/hexuint64.go +++ b/pkg/ethtypes/hexuint64.go @@ -18,11 +18,11 @@ package ethtypes import ( "context" - "encoding/json" "fmt" "strconv" "github.com/hyperledger/firefly-common/pkg/i18n" + "github.com/hyperledger/firefly-signer/internal/signermsgs" ) // HexUint64 is a positive integer - serializes to JSON as an 0x hex string (no leading zeros), and parses flexibly depending on the prefix (so 0x for hex, or base 10 for plain string / float64) @@ -40,22 +40,15 @@ func (h HexUint64) MarshalJSON() ([]byte, error) { } func (h *HexUint64) UnmarshalJSON(b []byte) error { - var i interface{} - _ = json.Unmarshal(b, &i) - switch i := i.(type) { - case float64: - *h = HexUint64(i) - return nil - case string: - i64, err := strconv.ParseUint(i, 0, 64) - if err != nil { - return fmt.Errorf("unable to parse integer: %s", i) - } - *h = HexUint64(i64) - return nil - default: - return fmt.Errorf("unable to parse integer from type %T", i) + bi, err := UnmarshalBigInt(b) + if err != nil { + return err + } + if !bi.IsUint64() { + return i18n.NewError(context.Background(), signermsgs.MsgInvalidUint64PrecisionLoss, b) } + *h = HexUint64(bi.Uint64()) + return nil } func (h HexUint64) Uint64() uint64 { diff --git a/pkg/ethtypes/hexuint64_test.go b/pkg/ethtypes/hexuint64_test.go index 3acdd785..17dad146 100644 --- a/pkg/ethtypes/hexuint64_test.go +++ b/pkg/ethtypes/hexuint64_test.go @@ -73,7 +73,7 @@ func TestHexUint64MissingBytes(t *testing.T) { }` err := json.Unmarshal([]byte(testData), &testStruct) - assert.Regexp(t, "unable to parse integer", err) + assert.Regexp(t, "FF22088", err) } func TestHexUint64BadType(t *testing.T) { @@ -115,7 +115,21 @@ func TestHexUint64BadNegative(t *testing.T) { }` err := json.Unmarshal([]byte(testData), &testStruct) - assert.Regexp(t, "parse", err) + assert.Regexp(t, "FF22090", err) +} + +func TestHexUint64BadTooLarge(t *testing.T) { + + testStruct := struct { + I1 HexUint64 `json:"i1"` + }{} + + testData := `{ + "i1": "18446744073709551616" + }` + + err := json.Unmarshal([]byte(testData), &testStruct) + assert.Regexp(t, "FF22090", err) } func TestHexUint64Constructor(t *testing.T) { diff --git a/pkg/ethtypes/integer_parsing.go b/pkg/ethtypes/integer_parsing.go new file mode 100644 index 00000000..ed1e844f --- /dev/null +++ b/pkg/ethtypes/integer_parsing.go @@ -0,0 +1,68 @@ +// Copyright © 2024 Kaleido, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ethtypes + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "math/big" + + "github.com/hyperledger/firefly-common/pkg/i18n" + "github.com/hyperledger/firefly-common/pkg/log" + "github.com/hyperledger/firefly-signer/internal/signermsgs" +) + +func BigIntegerFromString(ctx context.Context, s string) (*big.Int, error) { + // We use Go's default '0' base integer parsing, where `0x` means hex, + // no prefix means decimal etc. + i, ok := new(big.Int).SetString(s, 0) + if !ok { + f, _, err := big.ParseFloat(s, 10, 256, big.ToNearestEven) + if err != nil { + log.L(ctx).Errorf("Error parsing numeric string '%s': %s", s, err) + return nil, i18n.NewError(ctx, signermsgs.MsgInvalidNumberString, s) + } + i, accuracy := f.Int(i) + if accuracy != big.Exact { + // If we weren't able to decode without losing precision, return an error + return nil, i18n.NewError(ctx, signermsgs.MsgInvalidIntPrecisionLoss, s) + } + + return i, nil + } + return i, nil +} + +func UnmarshalBigInt(b []byte) (*big.Int, error) { + var i interface{} + d := json.NewDecoder(bytes.NewReader(b)) + d.UseNumber() + err := d.Decode(&i) + if err != nil { + return nil, err + } + switch i := i.(type) { + case json.Number: + return BigIntegerFromString(context.Background(), i.String()) + case string: + return BigIntegerFromString(context.Background(), i) + default: + return nil, fmt.Errorf("unable to parse integer from type %T", i) + } +} diff --git a/pkg/ethtypes/integer_parsing_test.go b/pkg/ethtypes/integer_parsing_test.go new file mode 100644 index 00000000..b5e4c06a --- /dev/null +++ b/pkg/ethtypes/integer_parsing_test.go @@ -0,0 +1,46 @@ +// Copyright © 2024 Kaleido, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ethtypes + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIntegerParsing(t *testing.T) { + ctx := context.Background() + + i, err := BigIntegerFromString(ctx, "1.0000000000000000000000001e+25") + assert.NoError(t, err) + assert.Equal(t, "10000000000000000000000001", i.String()) + + i, err = BigIntegerFromString(ctx, "10000000000000000000000000000001") + assert.NoError(t, err) + assert.Equal(t, "10000000000000000000000000000001", i.String()) + + i, err = BigIntegerFromString(ctx, "20000000000000000000000000000002") + assert.NoError(t, err) + assert.Equal(t, "20000000000000000000000000000002", i.String()) + + _, err = BigIntegerFromString(ctx, "0xGG") + assert.Regexp(t, "FF22088", err) + + _, err = BigIntegerFromString(ctx, "3.0000000000000000000000000000003") + assert.Regexp(t, "FF22089", err) +} From 2fe278d0353fcd5c792c4fccdbcfc8fa34d4caab Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Tue, 27 Aug 2024 14:52:35 -0400 Subject: [PATCH 2/2] Better errors Signed-off-by: Peter Broadhurst --- internal/signermsgs/en_error_messges.go | 1 + pkg/ethtypes/hexinteger.go | 2 +- pkg/ethtypes/hexinteger_test.go | 2 +- pkg/ethtypes/hexuint64.go | 2 +- pkg/ethtypes/hexuint64_test.go | 2 +- pkg/ethtypes/integer_parsing.go | 5 ++--- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/signermsgs/en_error_messges.go b/internal/signermsgs/en_error_messges.go index df5751ba..0e5a7002 100644 --- a/internal/signermsgs/en_error_messges.go +++ b/internal/signermsgs/en_error_messges.go @@ -107,4 +107,5 @@ var ( MsgInvalidNumberString = ffe("FF22088", "Invalid integer string '%s'") MsgInvalidIntPrecisionLoss = ffe("FF22089", "String %s cannot be converted to integer without losing precision") MsgInvalidUint64PrecisionLoss = ffe("FF22090", "String %s cannot be converted to a uint64 without losing precision") + MsgInvalidJSONTypeForBigInt = ffe("FF22091", "JSON parsed '%T' cannot be converted to an integer") ) diff --git a/pkg/ethtypes/hexinteger.go b/pkg/ethtypes/hexinteger.go index d18a1e92..c734c477 100644 --- a/pkg/ethtypes/hexinteger.go +++ b/pkg/ethtypes/hexinteger.go @@ -36,7 +36,7 @@ func (h HexInteger) MarshalJSON() ([]byte, error) { } func (h *HexInteger) UnmarshalJSON(b []byte) error { - bi, err := UnmarshalBigInt(b) + bi, err := UnmarshalBigInt(context.Background(), b) if err != nil { return err } diff --git a/pkg/ethtypes/hexinteger_test.go b/pkg/ethtypes/hexinteger_test.go index f473381e..8a3d6117 100644 --- a/pkg/ethtypes/hexinteger_test.go +++ b/pkg/ethtypes/hexinteger_test.go @@ -90,7 +90,7 @@ func TestHexIntegerBadType(t *testing.T) { }` err := json.Unmarshal([]byte(testData), &testStruct) - assert.Regexp(t, "unable to parse integer", err) + assert.Regexp(t, "FF22091", err) } func TestHexIntegerBadJSON(t *testing.T) { diff --git a/pkg/ethtypes/hexuint64.go b/pkg/ethtypes/hexuint64.go index bbfb02ac..a9356fbf 100644 --- a/pkg/ethtypes/hexuint64.go +++ b/pkg/ethtypes/hexuint64.go @@ -40,7 +40,7 @@ func (h HexUint64) MarshalJSON() ([]byte, error) { } func (h *HexUint64) UnmarshalJSON(b []byte) error { - bi, err := UnmarshalBigInt(b) + bi, err := UnmarshalBigInt(context.Background(), b) if err != nil { return err } diff --git a/pkg/ethtypes/hexuint64_test.go b/pkg/ethtypes/hexuint64_test.go index 17dad146..7f246f8f 100644 --- a/pkg/ethtypes/hexuint64_test.go +++ b/pkg/ethtypes/hexuint64_test.go @@ -87,7 +87,7 @@ func TestHexUint64BadType(t *testing.T) { }` err := json.Unmarshal([]byte(testData), &testStruct) - assert.Regexp(t, "unable to parse integer", err) + assert.Regexp(t, "FF22091", err) } func TestHexUint64BadJSON(t *testing.T) { diff --git a/pkg/ethtypes/integer_parsing.go b/pkg/ethtypes/integer_parsing.go index ed1e844f..f2f5427c 100644 --- a/pkg/ethtypes/integer_parsing.go +++ b/pkg/ethtypes/integer_parsing.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" "math/big" "github.com/hyperledger/firefly-common/pkg/i18n" @@ -49,7 +48,7 @@ func BigIntegerFromString(ctx context.Context, s string) (*big.Int, error) { return i, nil } -func UnmarshalBigInt(b []byte) (*big.Int, error) { +func UnmarshalBigInt(ctx context.Context, b []byte) (*big.Int, error) { var i interface{} d := json.NewDecoder(bytes.NewReader(b)) d.UseNumber() @@ -63,6 +62,6 @@ func UnmarshalBigInt(b []byte) (*big.Int, error) { case string: return BigIntegerFromString(context.Background(), i) default: - return nil, fmt.Errorf("unable to parse integer from type %T", i) + return nil, i18n.NewError(ctx, signermsgs.MsgInvalidJSONTypeForBigInt, i) } }