fix: F-2026-18197 | [Dual Defense] Nested Message Dispatch Bypasses EVM Ante for MsgEthereumTx - #317
Open
0xNilesh wants to merge 1 commit into
Open
fix: F-2026-18197 | [Dual Defense] Nested Message Dispatch Bypasses EVM Ante for MsgEthereumTx#3170xNilesh wants to merge 1 commit into
0xNilesh wants to merge 1 commit into
Conversation
Both are generic nested-message dispatchers that reach the message router after the ante handler has run, letting an MsgEthereumTx skip the EVM ante (F-2026-18197). Neither is used by Push. Adds a remove-group upgrade handler that prunes the group store.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
F-2026-18197 — remove the attack surface (chain side)
Ethereum signature, nonce and gas checks live only in the EVM ante handler;
x/vm'sKeeper.EthereumTxperforms none of them and assumes the ante already ran. The ante chain onlyruns over a tx's top-level messages, so any module that unpacks an embedded
sdk.Msgandre-dispatches it through the message router delivers an
MsgEthereumTxto the executor with theante already behind it — signature never checked, nonce force-set rather than verified
(
x/vm/keeper/state_transition.go,// - reset sender's nonce to msg.Nonce() before calling evm),so a victim-signed tx lifted off the mempool executes as the victim, repeatably.
Push had two such dispatchers wired. This PR deletes both rather than trying to enumerate the
message shapes they can carry. The companion PR hardens the sink.
Changes
x/groupremoved entirely — imports,GroupKeeper, store key, keeper construction, appmodule, and both module-ordering lists (
app/app.go). It is a generic proposal dispatcher(
MsgSubmitProposal/MsgExec) with zero usage anywhere in Push: no group, no group policyand no proposal is created by the chain, the universal client or any user flow, and
grouptypesappeared nowhere outside app wiring. The SDK 0.54 upgrade forces dropping it anyway, so this pulls
that work forward.
remove-groupupgrade handler (app/upgrades/remove-group/) —StoreUpgrades{Deleted: []string{group.StoreKey}}plusdelete(fromVM, group.ModuleName)soRunMigrationsdoes not try to migrate a module that is no longer registered. Registered inapp/upgrades.go, followingremove-utxverifier."stargate"dropped from both wasm capability lists (app/wasm.goforx/wasm,app/app.gofor the08-wasmlight-client VM). It was present only because the boilerplateAllCapabilities()returns everything wasmvm supports; it lets a contract emit an arbitraryencoded
sdk.Msg(CosmosMsg::Any/ Stargate) straight into the message router.Pre-flight
x/groupstate found locally. Both local exported states — the devnet genesis at~/.pchain/config/genesis.jsonandpush-smart-account-v1/state.json(localchain_9000-1,height 21) — carry the default empty group genesis:
group_seq: 0,groups: [],group_policies: [],proposals: [],votes: []. No group state exists anywhere in the repo'sconfigs either.
queried). If any group, group policy or open proposal exists on donut, the store deletion strands
it. Please confirm against donut before scheduling the upgrade — it is a single
q group groups-by-admin/ genesis-export check.stargate. The only wasm binary in the repo,interchaintest/contracts/cw_template.wasm, declaresrequires_iterator,requires_cosmwasm_1_1,1_2,1_3— norequires_stargate. Both local exported states showwasm.codes: []andwasm.contracts: [](nothing uploaded at all), andcode_upload_accessisEverybody.requires_stargateit will stop being instantiable.capabilities the contract declares in its exports. wasmd still wires
EncodeAnyMsgat runtime,so a contract that deliberately omits the
requires_stargateexport could in principle still emitan
Anymessage. That residual is exactly what the companion PR'sVerifySendercloses — thereason we are doing both halves rather than either alone. (Note the list also stops at
cosmwasm_1_4, socosmwasm_2_0is not available either.)Deliberately not done
Flipping
code_upload_accessoffEverybodyis a chain-policy change with no product decisionbehind it, it would break the existing
TestCosmWasmIntegrationinterchaintest which uploads froman ordinary account, and its marginal value is small once
stargateis gone and the sink ishardened. Easy to add later as a params change if wanted.
Hacken remediations
MessageRouter: DECLINED. This proposes redoingthe fix that already failed.
AuthzLimiterDecorator(app/ante/ante_cosmos.go) is thatdenylist — it is precisely the remediation Evmos shipped for GHSA-v6rw-hhgg-wc4x, the same bug
class. It held until two more dispatch modules were enabled underneath it, at which point it
silently stopped covering the surface. A denylist has to enumerate every dispatching module and
every nesting shape forever, and fails open when someone adds a module. Fixing the sink is
structural; and with
x/groupandstargategone there are no callers left to guard.wording was insufficient anyway: group proposals are stored and executed later, so an ante-only
check misses already-stored proposals — it would also have been needed at execution time.)
ante-skipping path exists.
Tests
app.TestGroupModuleNotWiredMsgSubmitProposal/MsgExec/MsgCreateGroup*unroutable on the msg service router and unresolvable in the interface registry (so tx decoding fails)app.TestWasmStargateCapabilityDisabledstargateabsent from bothAllCapabilities()and the08-wasmlisttxpolicy.TestGaslessMsgTypesExcludeEthereumTxMsgEthereumTxis not gasless, bare or nested inauthz.MsgExec;MsgExecutePayloadstill isintegrationtest.TestGaslessExecutePayloadWithModuleSenderMsgExecutePayloadstill executes end to end, and the uexecutor module account has no pubkey (so it could never sign anMsgEthereumTx)Results —
./app/,./app/ante/...,./app/decorators/...,./app/txpolicy/...,./app/upgrades/...,./test/...all green;go build ./...clean.The gasless invariant guard was additionally run against the patched EVM (local
replace github.com/cosmos/evm => ../push-chain-evmpointing at the companion branch): the wholetest/integration/uexecutorpackage passes, includingTestExecutePayloadandTestGaslessExecutePayloadWithModuleSender. Thereplacewas reverted before committing — see thefollow-up below.
Pre-existing failure unrelated to this PR:
app.TestBlockedAddrs/Setup(t)panic withunknown chain id: testingwhen theapppackage's tests are run without another test havinginitialised the global EVM configurator first (
const chainID = "testing"intest_helpers.govsChainID = "localchain_9000-1"). The new tests use the deterministic constructor to avoid it.Follow-up
Once the companion PR merges, bump this repo's
github.com/cosmos/evmreplace pin so the sink fixactually ships with the chain.
Companion PR (EVM side, hardens the sink with
VerifySender): pushchain/push-chain-evm#41