Problem: Some Web3 RPC Handlers could panic

Closes: evmos#701

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

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

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

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/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

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) {
-	txData := NewTxDataFromTx(tx)
+func (msg *MsgEthereumTx) FromEthereumTx(tx *ethtypes.Transaction) error {
+	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 nil
+	return msg.FromEthereumTx(tx)
 }
 
 // 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"
@@ -183,3 +185,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,11 @@ 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 i.BitLen() > maxBitLen {
+		return sdk.NewInt(0), errors.New("SafeNewIntFromBigInt() out of bound") // nolint
+	}
+	return sdk.NewIntFromBigInt(i), nil
+}