evmos · fedekunze · Nov 8, 2021 · Nov 8, 2021
@@ -41,6 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (evm, ante) [tharsis#620](https:/tharsis/ethermint/pull/620) Add fee market field to EVM `Keeper` and `AnteHandler`.
 * (all) [tharsis#231](https:/tharsis/ethermint/pull/231) Bump go-ethereum version to [`v1.10.9`](https:/ethereum/go-ethereum/releases/tag/v1.10.9)
+* (ante) [tharsis#703](https:/tharsis/ethermint/pull/703) Fix some fields in transaction are not authenticated by signature.
 
 ### Features
 
@@ -397,3 +398,5 @@ corresponding Ethereum API namespace:
 * (`x/evm`) [\#319](https:/cosmos/ethermint/pull/319) Fix `SetBlockHash` that was setting the incorrect height during `BeginBlock`.
 * (`x/evm`) [\#176](https:/cosmos/ethermint/issues/176) Updated Web3 transaction hash from using RLP hash. Now all transaction hashes exposed are amino hashes:
  * Removes `Hash()` (RLP) function from `MsgEthereumTx` to avoid confusion or misuse in future.
+
+
@@ -59,7 +59,7 @@ func NewAnteHandler(
  authante.NewMempoolFeeDecorator(),
  authante.NewTxTimeoutHeightDecorator(),
  authante.NewValidateMemoDecorator(ak),
- NewEthValidateBasicDecorator(),
+ NewEthValidateBasicDecorator(evmKeeper),
  NewEthSigVerificationDecorator(evmKeeper),
  NewEthAccountVerificationDecorator(ak, bankKeeper, evmKeeper),
  NewEthNonceVerificationDecorator(ak),

@@ -38,7 +38,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedContractTx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedContractTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedContractTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
  return tx
  },
  false, false, true,
@@ -49,7 +49,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedContractTx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedContractTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedContractTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
  return tx
  },
  true, false, true,
@@ -60,7 +60,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedContractTx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedContractTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedContractTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
  return tx
  },
  false, true, true,
@@ -71,7 +71,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedTx, privKey, 1, false)
  return tx
  },
  false, false, true,
@@ -82,7 +82,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 2, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedTx, privKey, 1, false)
  return tx
  },
  true, false, true,
@@ -93,7 +93,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 3, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedTx, privKey, 1, false)
  return tx
  }, false, true, true,
  },
@@ -125,11 +125,87 @@ func (suite AnteTestSuite) TestAnteHandler() {
  signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 5, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
  signedTx.From = addr.Hex()
 
- txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, true)
+ txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
  txBuilder.SetMemo(strings.Repeat("*", 257))
  return txBuilder.GetTx()
  }, true, false, false,
  },
+ // Based on EVMBackend.SendTransaction, for cosmos tx, forcing null for some fields except ExtensionOptions, Fee, MsgEthereumTx
+ // should be part of consensus
+ {
+ "fail - DeliverTx (cosmos tx signed)",
+ func() sdk.Tx {
+ nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
+ suite.Require().NoError(err)
+ signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+ signedTx.From = addr.Hex()
+
+ tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ return tx
+ }, false, false, false,
+ },
+ {
+ "fail - DeliverTx (cosmos tx with memo)",
+ func() sdk.Tx {
+ nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
+ suite.Require().NoError(err)
+ signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+ signedTx.From = addr.Hex()
+
+ txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
+ txBuilder.SetMemo("memo for cosmos tx not allowed")
+ return txBuilder.GetTx()
+ }, false, false, false,
+ },
+ {
+ "fail - DeliverTx (cosmos tx with timeoutheight)",
+ func() sdk.Tx {
+ nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
+ suite.Require().NoError(err)
+ signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+ signedTx.From = addr.Hex()
+
+ txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
+ txBuilder.SetTimeoutHeight(10)
+ return txBuilder.GetTx()
+ }, false, false, false,
+ },
+ {
+ "fail - DeliverTx (invalid fee amount)",
+ func() sdk.Tx {
+ nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
+ suite.Require().NoError(err)
+ signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+ signedTx.From = addr.Hex()
+
+ txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
+
+ txData, err := evmtypes.UnpackTxData(signedTx.Data)
+ suite.Require().NoError(err)
+
+ expFee := txData.Fee()
+ invalidFee := new(big.Int).Add(expFee, big.NewInt(1))
+ invalidFeeAmount := sdk.Coins{sdk.NewCoin(evmtypes.DefaultEVMDenom, sdk.NewIntFromBigInt(invalidFee))}
+ txBuilder.SetFeeAmount(invalidFeeAmount)
+ return txBuilder.GetTx()
+ }, false, false, false,
+ },
+ {
+ "fail - DeliverTx (invalid fee gaslimit)",
+ func() sdk.Tx {
+ nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
+ suite.Require().NoError(err)
+ signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+ signedTx.From = addr.Hex()
+
+ txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
+
+ expGasLimit := signedTx.GetGas()
+ invalidGasLimit := expGasLimit + 1
+ txBuilder.SetGasLimit(invalidGasLimit)
+ return txBuilder.GetTx()
+ }, false, false, false,
+ },
  }
 
  for _, tc := range testCases {
@@ -188,7 +264,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedContractTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedContractTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
  return tx
  },
  false, false, true,
@@ -210,7 +286,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedContractTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedContractTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
  return tx
  },
  true, false, true,
@@ -232,7 +308,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedContractTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedContractTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
  return tx
  },
  false, true, true,
@@ -255,7 +331,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedTx, privKey, 1, false)
  return tx
  },
  false, false, true,
@@ -278,7 +354,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedTx, privKey, 1, false)
  return tx
  },
  true, false, true,
@@ -301,7 +377,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedTx.From = addr.Hex()
 
- tx := suite.CreateTestTx(signedTx, privKey, 1, true)
+ tx := suite.CreateTestTx(signedTx, privKey, 1, false)
  return tx
  }, false, true, true,
  },
@@ -369,7 +445,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
  )
  signedTx.From = addr.Hex()
 
- txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, true)
+ txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
  txBuilder.SetMemo(strings.Repeat("*", 257))
  return txBuilder.GetTx()
  }, true, false, false,

@@ -8,6 +8,7 @@ import (
 
  sdk "github.com/cosmos/cosmos-sdk/types"
  sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+ tx "github.com/cosmos/cosmos-sdk/types/tx"
  authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
 
  ethermint "github.com/tharsis/ethermint/types"
@@ -35,6 +36,10 @@ type EVMKeeper interface {
  ) (sdk.Coins, error)
 }
 
+type protoTxProvider interface {
+ GetProtoTx() *tx.Tx
+}
+
 // EthSigVerificationDecorator validates an ethereum signatures
 type EthSigVerificationDecorator struct {
  evmKeeper EVMKeeper
@@ -534,11 +539,15 @@ func (issd EthIncrementSenderSequenceDecorator) AnteHandle(ctx sdk.Context, tx s
 }
 
 // EthValidateBasicDecorator is adapted from ValidateBasicDecorator from cosmos-sdk, it ignores ErrNoSignatures
-type EthValidateBasicDecorator struct{}
+type EthValidateBasicDecorator struct {
+ evmKeeper EVMKeeper
+}
 
 // NewEthValidateBasicDecorator creates a new EthValidateBasicDecorator
-func NewEthValidateBasicDecorator() EthValidateBasicDecorator {
- return EthValidateBasicDecorator{}
+func NewEthValidateBasicDecorator(ek EVMKeeper) EthValidateBasicDecorator {
+ return EthValidateBasicDecorator{
+ evmKeeper: ek,
+ }
 }
 
 // AnteHandle handles basic validation of tx
@@ -554,6 +563,87 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
  return ctx, stacktrace.Propagate(err, "tx basic validation failed")
  }
 
+ // For eth type cosmos tx, some fields should be veified as zero values,
+ // since we will only verify the signature against the hash of the MsgEthereumTx.Data
+ if wrapperTx, ok := tx.(protoTxProvider); ok {
+ protoTx := wrapperTx.GetProtoTx()
+ body := protoTx.Body
+ if body.Memo != "" || body.TimeoutHeight != uint64(0) || len(body.NonCriticalExtensionOptions) > 0 {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest,
+ "for eth tx body Memo TimeoutHeight NonCriticalExtensionOptions should be empty"),
+ "invalid data in tx",
+ )
+ }
+
+ if len(body.ExtensionOptions) != 1 {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "for eth tx length of ExtensionOptions should be 1"),
+ "invalid data in tx",
+ )
+ }
+
+ if len(protoTx.GetMsgs()) != 1 {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "only 1 ethereum msg supported per tx"),
+ "",
+ )
+ }
+ msg := protoTx.GetMsgs()[0]
+ msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx)
+ if !ok {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type %T, expected %T", tx, (*evmtypes.MsgEthereumTx)(nil)),
+ "failed to cast transaction",
+ )
+ }
+ ethGasLimit := msgEthTx.GetGas()
+
+ txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
+ if err != nil {
+ return ctx, stacktrace.Propagate(err, "failed to unpack MsgEthereumTx Data")
+ }
+ params := vbd.evmKeeper.GetParams(ctx)
+ ethFeeAmount := sdk.Coins{sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(txData.Fee()))}
+
+ authInfo := protoTx.AuthInfo
+ if len(authInfo.SignerInfos) > 0 {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "for eth tx AuthInfo SignerInfos should be empty"),
+ "invalid data in tx",
+ )
+ }
+
+ if authInfo.Fee.Payer != "" || authInfo.Fee.Granter != "" {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "for eth tx AuthInfo Fee payer and granter should be empty"),
+ "invalid data in tx",
+ )
+ }
+
+ if !authInfo.Fee.Amount.IsEqual(ethFeeAmount) {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid eth tx AuthInfo Fee Amount"),
+ "invalid data in tx",
+ )
+ }
+
+ if authInfo.Fee.GasLimit != ethGasLimit {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid eth tx AuthInfo Fee GasLimit"),
+ "invalid data in tx",
+ )
+ }
+
+ sigs := protoTx.Signatures
+ if len(sigs) > 0 {
+ return ctx, stacktrace.Propagate(
+ sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "for eth tx Signatures should be empty"),
+ "invalid data in tx",
+ )
+ }
+ }
+
  return next(ctx, tx, simulate)
 }