Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@

### BUG FIXES

- Verify the Ethereum sender in `Keeper.EthereumTx` before applying the transaction. The EVM ante
handler only runs over a tx's top-level messages, so an `MsgEthereumTx` nested inside a
dispatching module (`x/authz`, `x/group`, `x/gov`, a CosmWasm stargate/`Any` message, ICA) reached
the executor with its signature never checked, letting a victim-signed transaction be replayed as
the victim. `VerifySender` is now required before `ApplyTransaction`, so `From` is always the
ECDSA-recovered signer regardless of how the message arrived.
- [\#690](https://github.com/cosmos/evm/pull/690) Fix Ledger hardware wallet support for coin type 60.
- [\#769](https://github.com/cosmos/evm/pull/769) Fix erc20 ibc middleware to not to validate sender address format.
- [\#790](https://github.com/cosmos/evm/pull/790) fix panic in historical query due to missing EvmCoinInfo.
Expand Down
102 changes: 102 additions & 0 deletions tests/integration/x/vm/test_msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
sdkmath "cosmossdk.io/math"

sdktypes "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
)
Expand Down Expand Up @@ -260,3 +261,104 @@ func (s *KeeperTestSuite) TestRegisterPreinstalls() {
s.Require().NoError(err)
}
}

// TestEthereumTxSenderVerification is the regression guard for F-2026-18197
// (nested message dispatch bypasses the EVM ante for MsgEthereumTx).
//
// The EVM ante handler only runs over the top-level messages of a tx. A module
// that unpacks and re-dispatches an embedded sdk.Msg (x/authz MsgExec, x/group
// MsgSubmitProposal/MsgExec, x/gov proposals, a CosmWasm stargate/Any message,
// ICA) hands the nested message to this msg server with the ante already
// behind it, so the signature was never checked. Keeper.EthereumTx therefore
// verifies the sender itself: the declared From must be the ECDSA signer of the
// raw transaction, whatever route the message took to get here.
func (s *KeeperTestSuite) TestEthereumTxSenderVerification() {
testCases := []struct {
name string
mutate func(msg *types.MsgEthereumTx)
expErr bool
}{
{
// The ante already ran VerifySender for this path, so the keeper
// check is a redundant no-op and normal traffic is unaffected.
name: "pass - untouched victim-signed tx (normal JSON-RPC path)",
mutate: func(*types.MsgEthereumTx) {},
expErr: false,
},
{
// The attack: take a victim-signed tx off the mempool and push it
// through a nested dispatcher, declaring the attacker (or a group
// policy / authz grantee account) as the sender so the outer
// permission check passes. Recovery still yields the victim.
name: "fail - From spoofed to a different account",
mutate: func(msg *types.MsgEthereumTx) {
msg.From = s.Keyring.GetAddr(1).Bytes()
},
expErr: true,
},
{
// A module account is derived from a name, never from a key pair,
// so no signature can ever recover to it. This is what makes it a
// design invariant that module-driven EVM calls go through
// ApplyMessage* directly and never through this msg server.
name: "fail - From spoofed to a module account",
mutate: func(msg *types.MsgEthereumTx) {
msg.From = authtypes.NewModuleAddress(govtypes.ModuleName).Bytes()
},
expErr: true,
},
{
name: "fail - From cleared",
mutate: func(msg *types.MsgEthereumTx) {
msg.From = nil
},
expErr: true,
},
}

s.Run("fail - empty raw transaction", func() {
s.SetupTest()
// Raw unmarshals to a nil transaction when the field is empty, and
// ValidateBasic lives in the same ante path a nested message skips.
_, err := s.Network.App.GetEVMKeeper().EthereumTx(s.Network.GetContext(), &types.MsgEthereumTx{})
s.Require().Error(err)
s.Require().ErrorIs(err, errortypes.ErrInvalidRequest)
})

for _, tc := range testCases {
s.Run(tc.name, func() {
s.SetupTest()

victim := s.Keyring.GetKey(0)
recipient := s.Keyring.GetAddr(1)
tx, err := s.Factory.GenerateSignedEthTx(victim.Priv, types.EvmTxArgs{
To: &recipient,
Amount: big.NewInt(1e10),
})
s.Require().NoError(err)

msg := tx.GetMsgs()[0].(*types.MsgEthereumTx)
s.Require().Equal(victim.Addr.Bytes(), msg.From, "sanity: tx must be signed by the victim")
tc.mutate(msg)

ctx := s.Network.GetContext()
res, err := s.Network.App.GetEVMKeeper().EthereumTx(ctx, msg)

if !tc.expErr {
s.Require().NoError(err)
s.Require().False(res.Failed())
return
}

s.Require().Error(err)
s.Require().ErrorIs(err, errortypes.ErrorInvalidSigner)
s.Require().Contains(err.Error(), "signature verification failed")
// The transaction must not have been applied at all.
s.Require().Nil(res)
s.Require().False(
utils.ContainsEventType(ctx.EventManager().Events().ToABCIEvents(), types.EventTypeEthereumTx),
"a rejected tx must not emit an ethereum_tx event",
)
})
}
}
34 changes: 34 additions & 0 deletions x/vm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import (
"context"
"encoding/hex"
"fmt"
"math/big"
"strconv"

"github.com/hashicorp/go-metrics"

ethtypes "github.com/ethereum/go-ethereum/core/types"

cmttypes "github.com/cometbft/cometbft/types"

"github.com/cosmos/evm/x/vm/types"
Expand All @@ -17,6 +20,7 @@ import (

"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
)

Expand All @@ -30,6 +34,36 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t
ctx := sdk.UnwrapSDKContext(goCtx)

tx := msg.AsTransaction()
// MsgEthereumTx.Raw unmarshals to a nil transaction when the raw field is
// empty, and ValidateBasic is part of the same ante path that a nested
// message skips, so the executor cannot assume it ran. Fail cleanly instead
// of dereferencing nil below.
if tx == nil {
return nil, errorsmod.Wrap(errortypes.ErrInvalidRequest, "invalid transaction: raw ethereum transaction is empty")
}

// Verify that msg.From really is the ECDSA signer of the transaction.
//
// This is normally established by the EVM ante handler
// (ante/evm/05_signature_verification.go), but the ante chain only runs over
// the *top-level* messages of a tx. Any module that unpacks an embedded
// sdk.Msg and re-dispatches it through the message router - x/authz MsgExec,
// x/group MsgSubmitProposal/MsgExec, x/gov proposals, a CosmWasm
// stargate/Any message, ICA - delivers the nested message here with the ante
// already behind it, so nothing has checked the signature. An attacker could
// then take any victim-signed transaction off the mempool and have it
// executed as the victim. Checking here means the executor never trusts an
// unverified From, whatever route the message took to reach it.
//
// Cost on the normal path is ~zero: msg.AsTransaction() hands back the same
// *ethtypes.Transaction the ante verified, and go-ethereum caches the
// recovered sender on it per signer, so this is a cache hit rather than a
// second ECDSA recovery.
signer := ethtypes.MakeSigner(types.GetEthChainConfig(), big.NewInt(ctx.BlockHeight()), uint64(ctx.BlockTime().Unix())) //#nosec G115 -- int overflow is not a concern here
if err := msg.VerifySender(signer); err != nil {
return nil, errorsmod.Wrapf(errortypes.ErrorInvalidSigner, "signature verification failed: %s", err.Error())
}

txIndex := k.GetTxIndexTransient(ctx)

labels := []metrics.Label{
Expand Down
Loading