fix: Web3 RPC handlers panic (#702)

* Problem: Some Web3 RPC Handlers could panic

Closes: #701

Solution:
- return error rather than panic when decoding invalid tx

* add validation rules

* changelog

CHANGELOG.md

@@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (rpc) [tharsis#661](https://github.com/tharsis/ethermint/pull/661) Fix OOM bug when creating too many filters using JSON-RPC.
 * (evm) [tharsis#660](https://github.com/tharsis/ethermint/pull/660) Fix `nil` pointer panic in `ApplyNativeMessage`.
 * (evm, test) [tharsis#649](https://github.com/tharsis/ethermint/pull/649) Test DynamicFeeTx.
+* (evm) [tharsis#702](https://github.com/tharsis/ethermint/pull/702) Fix panic in web3 RPC handlers 
 
 ### Improvements
 

encoding/config.go

@@ -68,7 +68,10 @@ func (g txConfig) TxDecoder() sdk.TxDecoder {
 		err := tx.UnmarshalBinary(txBytes)
 		if err == nil {
 			msg := &evmtypes.MsgEthereumTx{}
-			msg.FromEthereumTx(tx)
+			err := msg.FromEthereumTx(tx)
+			if err != nil {
+				return nil, err
+			}
 			return msg, nil
 		}
 

x/evm/types/access_list_tx.go

@@ -10,7 +10,7 @@ import (
 	"github.com/tharsis/ethermint/types"
 )
 
-func newAccessListTx(tx *ethtypes.Transaction) *AccessListTx {
+func newAccessListTx(tx *ethtypes.Transaction) (*AccessListTx, error) {
 	txData := &AccessListTx{
 		Nonce:    tx.Nonce(),
 		Data:     tx.Data(),
@@ -23,12 +23,18 @@ func newAccessListTx(tx *ethtypes.Transaction) *AccessListTx {
 	}
 
 	if tx.Value() != nil {
-		amountInt := sdk.NewIntFromBigInt(tx.Value())
+		amountInt, err := SafeNewIntFromBigInt(tx.Value())
+		if err != nil {
+			return nil, err
+		}
 		txData.Amount = &amountInt
 	}
 
 	if tx.GasPrice() != nil {
-		gasPriceInt := sdk.NewIntFromBigInt(tx.GasPrice())
+		gasPriceInt, err := SafeNewIntFromBigInt(tx.GasPrice())
+		if err != nil {
+			return nil, err
+		}
 		txData.GasPrice = &gasPriceInt
 	}
 
@@ -38,7 +44,7 @@ func newAccessListTx(tx *ethtypes.Transaction) *AccessListTx {
 	}
 
 	txData.SetSignatureValues(tx.ChainId(), v, r, s)
-	return txData
+	return txData, nil
 }
 
 // TxType returns the tx type
@@ -177,6 +183,9 @@ func (tx AccessListTx) Validate() error {
 	if gasPrice == nil {
 		return sdkerrors.Wrap(ErrInvalidGasPrice, "cannot be nil")
 	}
+	if !IsValidInt256(gasPrice) {
+		return sdkerrors.Wrap(ErrInvalidGasPrice, "out of bound")
+	}
 
 	if gasPrice.Sign() == -1 {
 		return sdkerrors.Wrapf(ErrInvalidGasPrice, "gas price cannot be negative %s", gasPrice)
@@ -187,6 +196,13 @@ func (tx AccessListTx) Validate() error {
 	if amount != nil && amount.Sign() == -1 {
 		return sdkerrors.Wrapf(ErrInvalidAmount, "amount cannot be negative %s", amount)
 	}
+	if !IsValidInt256(amount) {
+		return sdkerrors.Wrap(ErrInvalidAmount, "out of bound")
+	}
+
+	if !IsValidInt256(tx.Fee()) {
+		return sdkerrors.Wrap(ErrInvalidGasFee, "out of bound")
+	}
 
 	if tx.To != "" {
 		if err := types.ValidateAddress(tx.To); err != nil {

x/evm/types/dynamic_fee_tx.go

@@ -12,7 +12,7 @@ import (
 	"github.com/tharsis/ethermint/types"
 )
 
-func newDynamicFeeTx(tx *ethtypes.Transaction) *DynamicFeeTx {
+func newDynamicFeeTx(tx *ethtypes.Transaction) (*DynamicFeeTx, error) {
 	txData := &DynamicFeeTx{
 		Nonce:    tx.Nonce(),
 		Data:     tx.Data(),
@@ -25,17 +25,26 @@ func newDynamicFeeTx(tx *ethtypes.Transaction) *DynamicFeeTx {
 	}
 
 	if tx.Value() != nil {
-		amountInt := sdk.NewIntFromBigInt(tx.Value())
+		amountInt, err := SafeNewIntFromBigInt(tx.Value())
+		if err != nil {
+			return nil, err
+		}
 		txData.Amount = &amountInt
 	}
 
 	if tx.GasFeeCap() != nil {
-		gasFeeCapInt := sdk.NewIntFromBigInt(tx.GasFeeCap())
+		gasFeeCapInt, err := SafeNewIntFromBigInt(tx.GasFeeCap())
+		if err != nil {
+			return nil, err
+		}
 		txData.GasFeeCap = &gasFeeCapInt
 	}
 
 	if tx.GasTipCap() != nil {
-		gasTipCapInt := sdk.NewIntFromBigInt(tx.GasTipCap())
+		gasTipCapInt, err := SafeNewIntFromBigInt(tx.GasTipCap())
+		if err != nil {
+			return nil, err
+		}
 		txData.GasTipCap = &gasTipCapInt
 	}
 
@@ -45,7 +54,7 @@ func newDynamicFeeTx(tx *ethtypes.Transaction) *DynamicFeeTx {
 	}
 
 	txData.SetSignatureValues(tx.ChainId(), v, r, s)
-	return txData
+	return txData, nil
 }
 
 // TxType returns the tx type
@@ -201,6 +210,14 @@ func (tx DynamicFeeTx) Validate() error {
 		return sdkerrors.Wrapf(ErrInvalidGasCap, "gas fee cap cannot be negative %s", tx.GasFeeCap)
 	}
 
+	if !IsValidInt256(tx.GetGasTipCap()) {
+		return sdkerrors.Wrap(ErrInvalidGasCap, "out of bound")
+	}
+
+	if !IsValidInt256(tx.GetGasFeeCap()) {
+		return sdkerrors.Wrap(ErrInvalidGasCap, "out of bound")
+	}
+
 	if tx.GasFeeCap.LT(*tx.GasTipCap) {
 		return sdkerrors.Wrapf(
 			ErrInvalidGasCap, "max priority fee per gas higher than max fee per gas (%s > %s)",
@@ -208,11 +225,18 @@ func (tx DynamicFeeTx) Validate() error {
 		)
 	}
 
+	if !IsValidInt256(tx.Fee()) {
+		return sdkerrors.Wrap(ErrInvalidGasFee, "out of bound")
+	}
+
 	amount := tx.GetValue()
 	// Amount can be 0
 	if amount != nil && amount.Sign() == -1 {
 		return sdkerrors.Wrapf(ErrInvalidAmount, "amount cannot be negative %s", amount)
 	}
+	if !IsValidInt256(amount) {
+		return sdkerrors.Wrap(ErrInvalidAmount, "out of bound")
+	}
 
 	if tx.To != "" {
 		if err := types.ValidateAddress(tx.To); err != nil {

x/evm/types/dynamic_fee_tx_test.go

@@ -60,7 +60,8 @@ func (suite *TxDataTestSuite) TestNewDynamicFeeTx() {
 		},
 	}
 	for _, tc := range testCases {
-		tx := newDynamicFeeTx(tc.tx)
+		tx, err := newDynamicFeeTx(tc.tx)
+		suite.Require().NoError(err)
 
 		suite.Require().NotEmpty(tx)
 		suite.Require().Equal(uint8(2), tx.TxType())

x/evm/types/errors.go

@@ -25,6 +25,7 @@ const (
 	codeErrCallDisabled
 	codeErrInvalidAmount
 	codeErrInvalidGasPrice
+	codeErrInvalidGasFee
 	codeErrVMExecution
 	codeErrInvalidRefund
 	codeErrInconsistentGas
@@ -72,6 +73,9 @@ var (
 	// ErrInvalidGasPrice returns an error if an invalid gas price is provided to the tx.
 	ErrInvalidGasPrice = sdkerrors.Register(ModuleName, codeErrInvalidGasPrice, "invalid gas price")
 
+	// ErrInvalidGasFee returns an error if the tx gas fee is out of bound.
+	ErrInvalidGasFee = sdkerrors.Register(ModuleName, codeErrInvalidGasFee, "invalid gas fee")
+
 	// ErrVMExecution returns an error resulting from an error in EVM execution.
 	ErrVMExecution = sdkerrors.Register(ModuleName, codeErrVMExecution, "evm transaction execution failed")
 

x/evm/types/legacy_tx.go

@@ -3,14 +3,13 @@ package types
 import (
 	"math/big"
 
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/ethereum/go-ethereum/common"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/tharsis/ethermint/types"
 )
 
-func newLegacyTx(tx *ethtypes.Transaction) *LegacyTx {
+func newLegacyTx(tx *ethtypes.Transaction) (*LegacyTx, error) {
 	txData := &LegacyTx{
 		Nonce:    tx.Nonce(),
 		Data:     tx.Data(),
@@ -23,17 +22,23 @@ func newLegacyTx(tx *ethtypes.Transaction) *LegacyTx {
 	}
 
 	if tx.Value() != nil {
-		amountInt := sdk.NewIntFromBigInt(tx.Value())
+		amountInt, err := SafeNewIntFromBigInt(tx.Value())
+		if err != nil {
+			return nil, err
+		}
 		txData.Amount = &amountInt
 	}
 
 	if tx.GasPrice() != nil {
-		gasPriceInt := sdk.NewIntFromBigInt(tx.GasPrice())
+		gasPriceInt, err := SafeNewIntFromBigInt(tx.GasPrice())
+		if err != nil {
+			return nil, err
+		}
 		txData.GasPrice = &gasPriceInt
 	}
 
 	txData.SetSignatureValues(tx.ChainId(), v, r, s)
-	return txData
+	return txData, nil
 }
 
 // TxType returns the tx type
@@ -161,12 +166,21 @@ func (tx LegacyTx) Validate() error {
 	if gasPrice.Sign() == -1 {
 		return sdkerrors.Wrapf(ErrInvalidGasPrice, "gas price cannot be negative %s", gasPrice)
 	}
+	if !IsValidInt256(gasPrice) {
+		return sdkerrors.Wrap(ErrInvalidGasPrice, "out of bound")
+	}
+	if !IsValidInt256(tx.Fee()) {
+		return sdkerrors.Wrap(ErrInvalidGasFee, "out of bound")
+	}
 
 	amount := tx.GetValue()
 	// Amount can be 0
 	if amount != nil && amount.Sign() == -1 {
 		return sdkerrors.Wrapf(ErrInvalidAmount, "amount cannot be negative %s", amount)
 	}
+	if !IsValidInt256(amount) {
+		return sdkerrors.Wrap(ErrInvalidAmount, "out of bound")
+	}
 
 	if tx.To != "" {
 		if err := types.ValidateAddress(tx.To); err != nil {

x/evm/types/legacy_tx_test.go

@@ -29,7 +29,8 @@ func (suite *TxDataTestSuite) TestNewLegacyTx() {
 	}
 
 	for _, tc := range testCases {
-		tx := newLegacyTx(tc.tx)
+		tx, err := newLegacyTx(tc.tx)
+		suite.Require().NoError(err)
 
 		suite.Require().NotEmpty(tc.tx)
 		suite.Require().Equal(uint8(0), tx.TxType())

x/evm/types/msg.go

@@ -130,17 +130,21 @@ func newMsgEthereumTx(
 }
 
 // fromEthereumTx populates the message fields from the given ethereum transaction
-func (msg *MsgEthereumTx) FromEthereumTx(tx *ethtypes.Transaction) {
+func (msg *MsgEthereumTx) FromEthereumTx(tx *ethtypes.Transaction) error {
-	txData := NewTxDataFromTx(tx)
+	txData, err := NewTxDataFromTx(tx)
+	if err != nil {
+		return err
+	}
 
 	anyTxData, err := PackTxData(txData)
 	if err != nil {
-		panic(err)
+		return err
 	}
 
 	msg.Data = anyTxData
 	msg.Size_ = float64(tx.Size())
 	msg.Hash = tx.Hash().Hex()
+	return nil
 }
 
 // Route returns the route value of an MsgEthereumTx.
@@ -225,8 +229,7 @@ func (msg *MsgEthereumTx) Sign(ethSigner ethtypes.Signer, keyringSigner keyring.
 		return err
 	}
 
-	msg.FromEthereumTx(tx)
+	return msg.FromEthereumTx(tx)
-	return nil
 }
 
 // GetGas implements the GasTx interface. It returns the GasLimit of the transaction.

x/evm/types/msg_test.go

@@ -8,10 +8,12 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/tharsis/ethermint/crypto/ethsecp256k1"
 	"github.com/tharsis/ethermint/tests"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
+	ethtypes "github.com/ethereum/go-ethereum/core/types"
 )
 
 const invalidFromAddress = "0x0000"
@@ -59,6 +61,7 @@ func (suite *MsgsTestSuite) TestMsgEthereumTx_ValidateBasic() {
 	hundredInt := sdk.NewInt(100)
 	zeroInt := sdk.ZeroInt()
 	minusOneInt := sdk.NewInt(-1)
+	exp_2_255 := sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(2), big.NewInt(255), nil))
 
 	testCases := []struct {
 		msg        string
@@ -80,6 +83,7 @@ func (suite *MsgsTestSuite) TestMsgEthereumTx_ValidateBasic() {
 		{msg: "negative gas price - Legacy Tx", to: suite.to.Hex(), amount: &hundredInt, gasPrice: &minusOneInt, expectPass: false},
 		{msg: "zero gas price - Legacy Tx", to: suite.to.Hex(), amount: &hundredInt, gasPrice: &zeroInt, expectPass: true},
 		{msg: "invalid from address - Legacy Tx", to: suite.to.Hex(), amount: &hundredInt, gasPrice: &zeroInt, from: invalidFromAddress, expectPass: false},
+		{msg: "out of bound gas fee - Legacy Tx", to: suite.to.Hex(), amount: &hundredInt, gasPrice: &exp_2_255, expectPass: false},
 		{msg: "nil amount - AccessListTx", to: suite.to.Hex(), amount: nil, gasPrice: &hundredInt, accessList: &types.AccessList{}, chainID: &hundredInt, expectPass: true},
 		{msg: "negative amount - AccessListTx", to: suite.to.Hex(), amount: &minusOneInt, gasPrice: &hundredInt, accessList: &types.AccessList{}, chainID: nil, expectPass: false},
 		{msg: "nil gas price - AccessListTx", to: suite.to.Hex(), amount: &hundredInt, gasPrice: nil, accessList: &types.AccessList{}, chainID: &hundredInt, expectPass: false},
@@ -183,3 +187,80 @@ func (suite *MsgsTestSuite) TestMsgEthereumTx_Sign() {
 		}
 	}
 }
+
+func (suite *MsgsTestSuite) TestFromEthereumTx() {
+	privkey, _ := ethsecp256k1.GenerateKey()
+	ethPriv, err := privkey.ToECDSA()
+	suite.Require().NoError(err)
+
+	// 10^80 is more than 256 bits
+	exp_10_80 := new(big.Int).Mul(big.NewInt(1), new(big.Int).Exp(big.NewInt(10), big.NewInt(80), nil))
+
+	testCases := []struct {
+		msg        string
+		expectPass bool
+		buildTx    func() *ethtypes.Transaction
+	}{
+		{"success, normal tx", true, func() *ethtypes.Transaction {
+			tx := ethtypes.NewTransaction(
+				0,
+				common.BigToAddress(big.NewInt(1)),
+				big.NewInt(10),
+				21000, big.NewInt(0),
+				nil,
+			)
+			tx, err := ethtypes.SignTx(tx, types.NewEIP2930Signer(suite.chainID), ethPriv)
+			suite.Require().NoError(err)
+			return tx
+		}},
+		{"fail, value bigger than 256bits", false, func() *ethtypes.Transaction {
+			tx := ethtypes.NewTransaction(
+				0,
+				common.BigToAddress(big.NewInt(1)),
+				exp_10_80,
+				21000, big.NewInt(0),
+				nil,
+			)
+			tx, err := ethtypes.SignTx(tx, types.NewEIP2930Signer(suite.chainID), ethPriv)
+			suite.Require().NoError(err)
+			return tx
+		}},
+		{"fail, gas price bigger than 256bits", false, func() *ethtypes.Transaction {
+			tx := ethtypes.NewTransaction(
+				0,
+				common.BigToAddress(big.NewInt(1)),
+				big.NewInt(10),
+				21000, exp_10_80,
+				nil,
+			)
+			tx, err := ethtypes.SignTx(tx, types.NewEIP2930Signer(suite.chainID), ethPriv)
+			suite.Require().NoError(err)
+			return tx
+		}},
+	}
+
+	for _, tc := range testCases {
+		ethTx := tc.buildTx()
+		tx := &MsgEthereumTx{}
+		err := tx.FromEthereumTx(ethTx)
+		if tc.expectPass {
+			suite.Require().NoError(err)
+
+			// round-trip test
+			suite.assertEthTxEqual(tx.AsTransaction(), ethTx)
+		} else {
+			suite.Require().Error(err)
+		}
+	}
+}
+
+func (suite *MsgsTestSuite) assertEthTxEqual(tx1 *ethtypes.Transaction, tx2 *ethtypes.Transaction) {
+	suite.Require().Equal(tx1.Hash(), tx2.Hash())
+	suite.Require().Equal(tx1.Size(), tx2.Size())
+
+	bin1, err := tx1.MarshalBinary()
+	suite.Require().NoError(err)
+	bin2, err := tx2.MarshalBinary()
+	suite.Require().NoError(err)
+	suite.Require().Equal(bin1, bin2)
+}

x/evm/types/tx_data.go

@@ -40,18 +40,22 @@ type TxData interface {
 	Cost() *big.Int
 }
 
-func NewTxDataFromTx(tx *ethtypes.Transaction) TxData {
+func NewTxDataFromTx(tx *ethtypes.Transaction) (TxData, error) {
 	var txData TxData
+	var err error
 	switch tx.Type() {
 	case ethtypes.DynamicFeeTxType:
-		txData = newDynamicFeeTx(tx)
+		txData, err = newDynamicFeeTx(tx)
 	case ethtypes.AccessListTxType:
-		txData = newAccessListTx(tx)
+		txData, err = newAccessListTx(tx)
 	default:
-		txData = newLegacyTx(tx)
+		txData, err = newLegacyTx(tx)
+	}
+	if err != nil {
+		return nil, err
 	}
 
-	return txData
+	return txData, nil
 }
 
 // DeriveChainID derives the chain id from the given v parameter.

x/evm/types/utils.go

@@ -1,7 +1,9 @@
 package types
 
 import (
+	"errors"
 	"fmt"
+	"math/big"
 
 	"github.com/gogo/protobuf/proto"
 
@@ -11,6 +13,8 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 )
 
+const maxBitLen = 256
+
 var EmptyCodeHash = crypto.Keccak256(nil)
 
 // DecodeTxResponse decodes an protobuf-encoded byte slice into TxResponse
@@ -86,3 +90,16 @@ func BinSearch(lo, hi uint64, executable func(uint64) (bool, *MsgEthereumTxRespo
 	}
 	return hi, nil
 }
+
+// SafeNewIntFromBigInt constructs Int from big.Int, return error if more than 256bits
+func SafeNewIntFromBigInt(i *big.Int) (sdk.Int, error) {
+	if !IsValidInt256(i) {
+		return sdk.NewInt(0), errors.New("SafeNewIntFromBigInt() out of bound") // nolint
+	}
+	return sdk.NewIntFromBigInt(i), nil
+}
+
+// IsValidInt256 check the bound of 256 bit number
+func IsValidInt256(i *big.Int) bool {
+	return i == nil || i.BitLen() <= maxBitLen
+}