diff --git a/.codecov.yml b/.codecov.yml index 821b8c9..9fb7588 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -35,3 +35,4 @@ ignore: - "**/test_utils.go" - "**/module.go" - "x/ibc/testing" + - "**/contracts" diff --git a/x/evm/keeper/txutils.go b/x/evm/keeper/txutils.go index ac51787..c547f0d 100644 --- a/x/evm/keeper/txutils.go +++ b/x/evm/keeper/txutils.go @@ -215,7 +215,7 @@ func (u *TxUtils) ConvertCosmosTxToEthereumTx(ctx context.Context, sdkTx sdk.Tx) return nil, nil, nil } - fees := authTx.GetFee() + feeAmount := authTx.GetFee() params, err := u.Params.Get(ctx) if err != nil { return nil, nil, err @@ -225,7 +225,7 @@ func (u *TxUtils) ConvertCosmosTxToEthereumTx(ctx context.Context, sdkTx sdk.Tx) return nil, nil, err } - if !(len(fees) == 0 || (len(fees) == 1 && fees[0].Denom == params.FeeDenom)) { + if !(len(feeAmount) == 0 || (len(feeAmount) == 1 && feeAmount[0].Denom == params.FeeDenom)) { return nil, nil, nil } @@ -258,7 +258,7 @@ func (u *TxUtils) ConvertCosmosTxToEthereumTx(ctx context.Context, sdkTx sdk.Tx) } else if len(sigData.Signature) == 64 { v, r, s = []byte{}, sigData.Signature[:32], sigData.Signature[32:64] } else { - return nil, nil, nil + return nil, nil, types.ErrTxConversionFailed.Wrap("invalid signature length") } gasFeeCap, ok := new(big.Int).SetString(md.GasFeeCap, 10) @@ -271,6 +271,13 @@ func (u *TxUtils) ConvertCosmosTxToEthereumTx(ctx context.Context, sdkTx sdk.Tx) return nil, nil, err } + // check if the fee amount is correctly converted + computedFeeAmount := sdk.NewCoins(sdk.NewCoin(params.FeeDenom, math.NewIntFromBigInt(computeGasFeeAmount(gasFeeCap, gas, decimals)))) + if !feeAmount.Equal(computedFeeAmount) { + u.Logger(ctx).Error("fee amount manipulation detected", "expected", computedFeeAmount, "actual", feeAmount) + return nil, nil, types.ErrTxConversionFailed.Wrap("fee amount manipulation detected") + } + var to *common.Address var input []byte var value *big.Int @@ -397,13 +404,13 @@ func (u *TxUtils) IsEthereumTx(ctx context.Context, sdkTx sdk.Tx) (bool, error) return false, nil } - fees := authTx.GetFee() + feeAmount := authTx.GetFee() params, err := u.Params.Get(ctx) if err != nil { return false, err } - if !(len(fees) == 0 || (len(fees) == 1 && fees[0].Denom == params.FeeDenom)) { + if !(len(feeAmount) == 0 || (len(feeAmount) == 1 && feeAmount[0].Denom == params.FeeDenom)) { return false, nil } diff --git a/x/evm/keeper/txutils_test.go b/x/evm/keeper/txutils_test.go index 15e1cf9..20e1bb8 100644 --- a/x/evm/keeper/txutils_test.go +++ b/x/evm/keeper/txutils_test.go @@ -10,11 +10,11 @@ import ( "github.com/stretchr/testify/require" "cosmossdk.io/math" - authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" - + "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" coretypes "github.com/ethereum/go-ethereum/core/types" @@ -112,7 +112,8 @@ func Test_DynamicFeeTxConversion(t *testing.T) { }) authTx := sdkTx.(authsigning.Tx) - require.Equal(t, authTx.GetFee(), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewIntFromBigInt(feeAmount).AddRaw(1)))) + expectedFeeAmount := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewIntFromBigInt(feeAmount).AddRaw(1))) + require.Equal(t, authTx.GetFee(), expectedFeeAmount) sigs, err := authTx.GetSignaturesV2() require.NoError(t, err) @@ -137,6 +138,12 @@ func Test_DynamicFeeTxConversion(t *testing.T) { ethTx2, _, err := keeper.NewTxUtils(&input.EVMKeeper).ConvertCosmosTxToEthereumTx(ctx, sdkTx) require.NoError(t, err) EqualEthTransaction(t, signedTx, ethTx2) + + // manipulate the fee amount + txBuilder := sdkTx.(client.TxBuilder) + txBuilder.SetFeeAmount(expectedFeeAmount.Add(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1)))) + _, _, err = keeper.NewTxUtils(&input.EVMKeeper).ConvertCosmosTxToEthereumTx(ctx, txBuilder.GetTx()) + require.ErrorIs(t, err, types.ErrTxConversionFailed) } func Test_AccessTxConversion(t *testing.T) { @@ -298,7 +305,8 @@ func Test_AccessTxConversion(t *testing.T) { }) authTx = sdkTx.(authsigning.Tx) - require.Equal(t, authTx.GetFee(), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewIntFromBigInt(feeAmount).AddRaw(1)))) + expectedFeeAmount := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewIntFromBigInt(feeAmount).AddRaw(1))) + require.Equal(t, authTx.GetFee(), expectedFeeAmount) sigs, err = authTx.GetSignaturesV2() require.NoError(t, err) @@ -323,6 +331,12 @@ func Test_AccessTxConversion(t *testing.T) { ethTx2, _, err = keeper.NewTxUtils(&input.EVMKeeper).ConvertCosmosTxToEthereumTx(ctx, sdkTx) require.NoError(t, err) EqualEthTransaction(t, signedTx, ethTx2) + + // manipulate the fee amount + txBuilder := sdkTx.(client.TxBuilder) + txBuilder.SetFeeAmount(expectedFeeAmount.Add(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1)))) + _, _, err = keeper.NewTxUtils(&input.EVMKeeper).ConvertCosmosTxToEthereumTx(ctx, txBuilder.GetTx()) + require.ErrorIs(t, err, types.ErrTxConversionFailed) } func Test_LegacyTxConversion(t *testing.T) { @@ -394,7 +408,8 @@ func Test_LegacyTxConversion(t *testing.T) { }) authTx := sdkTx.(authsigning.Tx) - require.Equal(t, authTx.GetFee(), sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewIntFromBigInt(feeAmount).AddRaw(1)))) + expectedFeeAmount := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewIntFromBigInt(feeAmount).AddRaw(1))) + require.Equal(t, authTx.GetFee(), expectedFeeAmount) sigs, err := authTx.GetSignaturesV2() require.NoError(t, err) @@ -419,6 +434,12 @@ func Test_LegacyTxConversion(t *testing.T) { ethTx2, _, err := keeper.NewTxUtils(&input.EVMKeeper).ConvertCosmosTxToEthereumTx(ctx, sdkTx) require.NoError(t, err) EqualEthTransaction(t, signedTx, ethTx2) + + // manipulate the fee amount + txBuilder := sdkTx.(client.TxBuilder) + txBuilder.SetFeeAmount(expectedFeeAmount.Add(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(1)))) + _, _, err = keeper.NewTxUtils(&input.EVMKeeper).ConvertCosmosTxToEthereumTx(ctx, txBuilder.GetTx()) + require.ErrorIs(t, err, types.ErrTxConversionFailed) } func Test_IsEthereumTx(t *testing.T) { diff --git a/x/evm/types/errors.go b/x/evm/types/errors.go index faa1ccf..c4faaff 100644 --- a/x/evm/types/errors.go +++ b/x/evm/types/errors.go @@ -37,6 +37,7 @@ var ( ErrFailedToGetERC20WrapperAddr = errorsmod.Register(ModuleName, 28, "Failed to get ERC20 wrapper address") ErrInvalidNumRetainBlockHashes = errorsmod.Register(ModuleName, 29, "Invalid num retain block hashes") ErrExceedMaxRecursiveDepth = errorsmod.Register(ModuleName, 30, "Exceed max recursive depth") + ErrTxConversionFailed = errorsmod.Register(ModuleName, 31, "Tx conversion failed") ) func NewRevertError(revert []byte) error {