Skip to content

fix: F-2026-18187 | [Dual Defense] Gov Precompile getTallyResult Persists SDK Tally Vote Deletions - #42

Merged
0xNilesh merged 3 commits into
audit-fixesfrom
F-2026-18187
Aug 24, 2026
Merged

0xNilesh merged 3 commits into
audit-fixesfrom
F-2026-18187

Conversation

@0xNilesh

Copy link
Copy Markdown
Member

F-2026-18187 — a "query" that deletes votes, and commits

getTallyResult on the gov precompile is classified a query: it is absent from IsTransaction
(precompiles/gov/gov.go:148-154). That classification is load-bearing in exactly two places —
SetupABI's staticcall guard (readOnly && isTransaction → ErrWriteProtection) and RequiredGas.
Neither of them gates whether the call's state writes are kept.

What decides that is RunNativeAction → runNativeAction (precompiles/common/precompile.go:57-90),
which calls stateDB.CommitWithCacheCtx() unconditionally and never branches on query vs
transaction. So the precompile's whole cache context — everything the method wrote — rides out with
the enclosing EVM transaction.

Meanwhile getTallyResult → SDK TallyResult → Keeper.Tally → Votes.Remove
(x/gov/keeper/tally.go:117). Tallying deleting the votes it counts is not an SDK bug: it runs
once, terminally, in the gov EndBlocker, and its gRPC TallyResult query is safe because
queries run against a context that is never committed. The SDK's invariant is "tallying mutates,
but query paths never commit". The precompile takes that mutating query and runs it in a committing
context.

Net effect: an ordinary EVM transaction calling getTallyResult(proposalId) on a proposal in the
voting period deletes that proposal's votes, and the deletion commits. Permissionless,
repeatable, costs only gas. The gov precompile is live on donut.

The fix

GetTallyResult now runs the SDK query against a throwaway cache and discards its store writes:

cacheCtx, discardWrites := newDiscardedCacheContext(ctx)
defer discardWrites()

res, err := p.govQuerier.TallyResult(cacheCtx, queryTallyResultReq)

The vote deletions evaporate with the cache; the tally itself is still the one the SDK computes —
delegations, weighted votes, validator defaults — so nothing about the returned numbers changes.

Why it is not a bare ctx.CacheContext()

The intended two-liner was cacheCtx, _ := ctx.CacheContext(). That does not isolate anything
inside a precompile, and it was verified not to
— with it in place both regression tests below
still failed with the votes gone.

sdk.Context.CacheContext defers to the underlying multi store's CacheMultiStore(). On the SDK's
own stores that hands back a detached cache, and dropping the write function is enough. The store
the EVM runs precompiles on is x/vm/store/snapshotmulti, whose CacheMultiStore() is:

func (s *Store) CacheMultiStore() storetypes.CacheMultiStore {
	s.Snapshot()
	return s
}

It pushes a snapshot layer and returns the very same store. The layer stays live as
CurrentStore(), and Write() commits every layer top-to-bottom when the state DB commits — so the
deletions land regardless of the discarded write function.

newDiscardedCacheContext therefore pops the layer explicitly on that store (Snapshot() /
RevertToSnapshot() — the same mechanism runNativeAction already uses to roll a failed precompile
back), and falls back to plain CacheContext() on any other store. The snapshot/revert pair is
balanced, so the state DB's own snapshot bookkeeping is left exactly as it was found. This is the
same idea as the agreed approach, expressed in the vocabulary this multi store actually implements;
the change stays entirely inside GetTallyResult.

Tests

TestGetTallyResultDoesNotDeleteVotes (tests/integration/precompiles/gov/test_gov.go) drives the
precompile through Run and then commits the state DB, exactly as a delivered transaction does.
Setup: proposal 1 in the voting period with 3 stored votes — a weighted vote from the network's
only delegator split 0.4 Yes / 0.3 Abstain / 0.2 No / 0.1 NoWithVeto, plus two zero-power voters that
contribute nothing to the tally but are votes Keeper.Tally would delete all the same.

Run for both readOnly = false (normal committing CALL — the actual attack) and
readOnly = true (STATICCALL), asserting:

  • Headline: all 3 voters are still stored after the committing call.
  • Positive control: the returned tally is still correct across every option —
    1.2e18 / 0.9e18 / 0.6e18 / 0.3e18 — so mutation was not traded for wrong numbers.
  • Repeatability: a second call returns the same tally and the votes survive again (before the
    fix the second call would have returned all zeros).

Regression-detector check: with the fix reverted, both subtests fail on
getTallyResult must not delete the proposal's votes; restored, both pass.

Results:

  • evmd/tests/integration/precompiles/gov (full gov precompile suite, unit + Ginkgo integration) — ok
  • precompiles/gov — ok
  • x/vm/store/snapshotkv, x/vm/store/snapshotmulti — ok (untouched, run because the fix relies
    on their semantics)
  • go vet clean, gofmt clean.

Hacken's other recommendations

2 — return empty/partial for voting-period proposals: declined. A live tally is the whole point
of the call; returning nothing during the voting period is a functional regression, and it is
redundant once the query stops mutating.

3 — classify the mutating path as IsTransaction == true / reject when readOnly: declined as
written.
IsTransaction only feeds the staticcall guard and gas pricing. The attack is a normal
committing CALL
with readOnly == false, so this blocks the harmless staticcall and leaves the
harmful call untouched — the tests above confirm both paths reach the mutating SDK code.

The structural form of that idea — gating the commit on the classification in runNativeAction
instead of committing unconditionally — is the real class fix, but it changes behaviour for every
precompile's query methods and the framework is upstream's. Confirmed still present on cosmos/evm
main
(same unconditional pattern via FlushToCacheCtx), so it is being reported upstream rather
than forked here. Note this is not covered by ISA-2025-004 / GHSA-mjfq-3qr2-6g84 ("Partial
Precompile State Writes"), which is about atomicity on failure and was fixed with RunAtomic.

@0xNilesh
0xNilesh merged commit 3358532 into audit-fixes Aug 24, 2026
12 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant