Repository navigation
fix: F-2026-18786 | [Dual Defense] Unknown Selector Still Runs CommitWithCacheCtx Before ABI Fail - #48
Merged
Conversation
Stateful precompiles ran the full native-action preamble - including a state DB commit that replays the caller's dirty set - before SetupABI resolved the selector, and RequiredGas returns 0 for an unresolvable one so none of it was charged.
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.
Finding
F-2026-18786 — Unknown Selector Still Runs
CommitWithCacheCtxBefore ABI FailSeverity: Low (Impact 1 / Likelihood 2).
Every stateful precompile routes through
cmn.Precompile.RunNativeAction, which runs the wholeexpensive preamble before the ABI selector is ever resolved:
RequiredGas— charged by go-ethereum beforeRun— returns 0 for an unresolvable selector,with a comment stating the assumption that fails: "This should never happen since this method is
going to fail during Run." It does happen (four random bytes), and "during Run" is after the commit.
initialGasis also captured afterCommitWithCacheCtx, so the preamble is outside the metereven on the success path.
commitWithCtxruns withkeepDirty=true, so each of up to 20 precompile frames per tx re-replaysthe entire dirty set — a 20x multiplier.
Fix
Resolve the ABI method at the top of each concrete
Run, beforeRunNativeActionis entered.Feasible because each concrete precompile embeds
abi.ABIdirectly, unlike the sharedcmn.Precompilewrapper.Applied to the eight stateful precompiles that route through the wrapper:
bank,gov,staking,distribution,ics20,slashing, pluserc20andwerc20, which use the same wrapper andwere not covered by the original PoC.
The guard is fallback-aware, not a length check
The obvious form of this fix —
— is wrong for
werc20, whose ABI declares both afallbackand areceive.cmn.SetupABIhas a documented special case that
werc20.Executedepends on: empty calldata resolves toreceive/fallback, calldata shorter than a selector resolves tofallback, and an unknownselector resolves to
fallback— all three behaving likedeposit. There are twelve existingintegration specs covering exactly those paths.
So the guard goes through a new
cmn.ResolveMethod, which is the selector-resolution half ofSetupABIlifted out verbatim;SetupABInow calls it, so the early check and the in-action checkcannot drift apart. Errors are returned through
cmn.ReturnRevertError, matching whatRunNativeActiondoes today, so the revert reason and gas refund on the failure path are byte-identical.Live verification (local node only)
Re-ran the PoC harness (
Probe.soldirties n storage slots, then calls the target with0xdeadbeef; 20x amplification), on the same machine in the same session, against the unmodifiedbinary and against a
pchaindbuilt from this branch.receipt.gasUsedis unusable here (feemarketmin_gas_multiplier=0.5floors it togasLimit/2), so gas iseth_estimateGasand CPU iseth_callround-trip median latency.Dirty-set sweep,
bankminus no-op target, 20 frames:Gas is unchanged at every point — the fix costs nothing and charges nothing new. CPU cost, dead
linear before, is now flat and indistinguishable from the no-op baseline.
(The original PoC run measured +39.32 ms at n=1000 on a more loaded machine; the same-session
control above reproduces the same linear shape at +28.64 ms.)
Per-precompile map (n=500, k=20, garbage selector), Δms vs no-op:
0x…08000x…08010x…08020x…08040x…08050x…08060x…0100(stateless control)0x…0400(stateless control)0xEC…01(Push control)All six affected precompiles now sit on the
usigverifier/ no-op line.Valid dispatch on the live node is unaffected:
bank.balances,bank.totalSupplyandslashing.getParamsreturn real data, and theMaxPrecompileCallscap still readsk=1→1, 5→5, 19→19, 20→20, 21→20, 25→20, 40→20, 64→20 — identical to the pre-fix numbers.
Direct EOA→precompile calls with a garbage selector still revert with
status=0andgasUsedat thegasLimit/2floor, i.e. gas is refunded rather than consumed, confirming therevert semantics did not change.
Tests
New
*_selector_test.goin each of the eight precompile packages. They driveRunwith a nilStateDB: the early guard never looks atevm.StateDB, whereas the first thingRunNativeActiondoes is type-assert on it, so a revert reason of
not run in EVMproves the call reached thepreamble and any other reason proves it was rejected before it.
no method with idwerc20instead asserts that all of empty / short / unknown calldata still reach the nativeaction, because its
fallback/receivelegitimately handle themMutation-verified both ways:
bankfails 3 of its 4 subtests (should not contain "not run in EVM"),while the valid-selector subtest keeps passing;
len < 4 || MethodByIdtemplate onwerc20fails all four of itsfallback/receive subtests and 10 specs of the real
werc20integration suite.Suites run (
-tags=test): the wholeprecompiles/...tree green; integration suites for werc20(81/81 specs), erc20, slashing, ics20, gov, distribution and staking all green. The
bankintegration suite has 4 pre-existing failures (stale total-supply constants) that reproduce
identically on unmodified
audit-fixes.Declined recommendations
initialGasis capturedafter
CommitWithCacheCtx, so charging on the error path would not cover the preamble anyway.Making it correct means moving the gas baseline, which changes gas accounting for every
stateful precompile call including successful ones — far too much risk for an Impact 1 finding.
This PR deletes the wasted work instead of pricing it, so no accounting changes at all.
RequiredGasfloor. Deferred as the weaker half. It prices the waste ratherthan preventing it, and there is no honest value for it — the cost covers "a snapshot plus a
commit whose size depends on the caller's dirty set". Optional belt-and-braces after this fix.
Provenance
Inherited from cosmos/evm. Unfixed in every tag and on
upstream/main@3e646c43; notPush-authored. Push's own precompiles are unaffected because they already implement this pattern:
precompiles/usigverifier/usigverifier.go(0xEC…01) and theutxverifierprecompiles(
0x…CA/0x…CB) overrideRunand resolve the selector first — they are the in-repo precedentthis fix mirrors, and they were the PoC's flat controls.
Interaction with #47
#47 merges
developintoaudit-fixesforcosmos/evm v0.6.0 and also touches
precompiles/common/precompile.go, but only insiderunNativeAction(AddPrecompileFnloses itseventsargument,CommitWithCacheCtxbecomesFlushToCacheCtx). This PR's edits to that file areSetupABIand the newResolveMethodbeneathit, so the two should not conflict. The finding survives v0.6.0 unchanged —
FlushToCacheCtxis thesame replay, still ahead of selector resolution — so this fix stays correct and necessary after the
merge either way.