Skip to content

fix(security): complete GHSA Aug-2026 statedb hardening (v35/mainnet) - #36

Merged
skosito merged 8 commits into
release/v35from
sec/mainnet-v36-complete
Sep 3, 2026
Merged

skosito merged 8 commits into
release/v35from
sec/mainnet-v36-complete

Conversation

@skosito

@skosito skosito commented Sep 3, 2026 •

Copy link
Copy Markdown
Member

Completes the balance-hardening port for the fork line that node release/v36 (mainnet) pins.

Changes

  • SubBalance underflow guard + ParseAmount extended-denom handling — (pre-existing on this branch)
  • Module-account guard, gated on delta.Sign() != 0 — keeps x/fungible's module-account EVM calls (incl. the BeginBlock gas-stability-pool burn) working; verbatim upstream guard would break them — (pre-existing)
  • Snapshot locked balance on statedb account — (pre-existing, backport fix(statedb): snapshot locked balance on statedb account  cosmos/evm#1187)
  • AddBalance overflow guard — panic on uint256 overflow (NEW, v0.6.3/v0.7.3 hotfix)
  • StateDB.Commit() atomicity — stage the dirty set in a cache context so a late failure discards the whole commit, including precompile-staged writes (NEW)
  • x/ibc/callbacks onPacketTimeout — use cachedCtx, not the live ctx (three sibling call sites already use cachedCtx; this one didn't) (NEW)
  • TestCommitAtomicity regression test ported (NEW)

CI note

Advisory/private review — please run locally; do not rely on aggregate green (the fork's check_diff/test-solidity are red for unrelated reasons):

go test ./x/vm/statedb/... -tags test -count=1

TestCommitAtomicity is load-bearing: mutation-verified — reverting the Commit() change makes late failure discards precompile-staged writes too FAIL (the credited balance survives the error).


Note

High Risk
Touches core EVM–bank reconciliation, commit atomicity, and fund-blocking policy; mistakes could affect supply invariants, vesting correctness, or reintroduce statedb exploit paths.

Overview
Ports upstream cosmos/evm StateDB balance hardening into this fork: accounts now carry a locked-balance snapshot at load time, commit reconciles bank balances as spendable + locked via SetBalanceWithLocked / SetAccountBalance, and in-memory AddBalance / SubBalance panic on overflow and underflow.

StateDB.Commit() stages the full dirty set in a cache context (including folding journal state into the precompile cache) so a late SetAccount failure rolls back credits and precompile-staged writes. Mint/burn reconciliation is blocked for non-zero deltas to addresses on the chain’s BlockedAddr policy (not upstream’s blanket module-account ban), preserving module accounts such as x/fungible that may pay value through the EVM.

Supporting changes: LockedCoins plumbed through bank wrappers and precisebank; precompile ParseAmount sums base and extended EVM denoms; x/ibc/callbacks onPacketTimeout runs CallEVM on cachedCtx; ERC20 code registration reuses the existing statedb account. Regression coverage includes atomic commit, vesting delegation, and blocked-pool reconciliation tests.

Reviewed by Cursor Bugbot for commit feb2e93. Configure here.

mergify Bot and others added 7 commits August 21, 2026 07:15
…smos#1187) (cosmos#1190)

* fix(statedb): snapshot locked balance on statedb account  (cosmos#1187)

* snapshot locked balance on statedb account creation to calculate spendable + locked bank balance after locked balance changes via precompile

* lint formatting

* modify x/erc20 registering code hash to not silently drop existing accounts locked balacne

* update comments

* Update x/vm/keeper/statedb.go

Co-authored-by: Vlad J <vladjdk@gmail.com>

---------

Co-authored-by: Vlad J <vladjdk@gmail.com>
(cherry picked from commit 008c171)

* fix conflicts

* bump sol

---------

Co-authored-by: mattac21 <matt@cosmoslabs.io>
Co-authored-by: Vlad <vladjdk@gmail.com>
(cherry picked from commit ee96860)
* fix

* tests

---------

Co-authored-by: Eric Warehime <eric.warehime@gmail.com>
(cherry picked from commit 3524ebc)
…s#1176) (cosmos#1253)

* fix: harden statedb balance and event amount handling (cosmos#1176)

* fix: harden statedb balance and event amount handling

Guard StateDB balance subtraction against underflow and make precompile balance-event parsing denom-aware for base vs extended denom paths. Also add regression tests and document that only 18-decimal EVM gas-token chains are supported.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(vm): enforce 18-decimal coin configuration

Reject non-18 decimal EVM coin configs in both runtime and test configurators, align affected tests, and temporarily exclude precisebank packages from root unit-test targets until precisebank removal lands.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: remove obsolete precisebank test package filters

Now that contrib/x/precisebank is removed on main, package selection no longer needs explicit exclusions and can rely on the standard simulation/e2e filters.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: fix formatter ordering in scaling tests

Apply golangci formatter output for scaling tests so gci/gofumpt checks pass in CI.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test: align integration suites with 18-decimal-only config

Remove non-18-decimal integration cases and fee checks that now fail by design under enforced 18-decimal EVM coin configuration.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
(cherry picked from commit 264aa70)

# Conflicts:
#	README.md
#	x/vm/statedb/state_object.go
#	x/vm/types/denom_config.go
#	x/vm/wrappers/feemarket_test.go

* fix conflicts

* fix formatting

* remove the requirement that the EVM coin be configured with 18 decimals

* sum the base and extended denom amounts when parsing balance change events

---------

Co-authored-by: Alex | Cosmos Labs <alex@cosmoslabs.io>
Co-authored-by: Matt Acciai <matt@cosmoslabs.io>
(cherry picked from commit 82b3ef6)
Three follow-ups needed to make the cherry-picked upstream fixes correct
and non-breaking on this fork:

1. precompiles/common: the fork carried duplicate unexported copies of
   parseAmount/parseHexAddress in balance_handler.go, and cosmos#1253 patched
   only the exported ParseAmount in utils.go. The live path therefore
   stayed unpatched. Deduplicated onto the patched exported helpers.

2. precompiles/common/utils.go: the patched ParseAmount body uses
   big.Int but the import hunk did not apply; added "math/big".

3. x/vm/keeper: narrow the module-account guard to fire only on a
   non-zero delta. Upstream rejects module accounts unconditionally,
   which breaks any EVM call made from a module account -- ZetaChain's
   x/fungible issues those for ZRC20 deploys, gas-pool swaps and system
   contract calls, and they reach SetBalance with delta == 0. A zero
   delta mints and burns nothing, so the security property is unchanged.
   Covers the mint direction with a new test case.

Verified: TestCallEVMWithData failures are identical to the unpatched
base commit (pre-existing); all new upstream security tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… timeout ctx

Completes the GHSA Aug-2026 statedb hardening on the v35 fork line:
- AddBalance: panic on uint256 overflow (v0.6.3/v0.7.3 hotfix)
- StateDB.Commit(): stage the dirty set in a cache context so a late
  failure discards the whole commit (incl. precompile-staged writes)
- x/ibc/callbacks onPacketTimeout: use cachedCtx, not the live ctx
- port upstream TestCommitAtomicity regression test

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eceive policy

The narrowed guard from the previous commit still bricks x/fungible. Gating on
a non-zero delta covers module-initiated EVM calls that move no value (ZRC20
deploys, approvals, system-contract calls), but x/fungible also pays real value
out of its own module account: SetupChainGasCoinAndPool sends native ZETA into
the gas pool through a payable addLiquidityETH, which is a genuine non-zero
delta and was rejected.

Caught by node's x/fungible/migrations/v4 suite, which the previous verification
did not run -- only evm-repo tests were exercised, and TestCallEVMWithData
covers the delta == 0 shape rather than the value-moving one. The live blast
radius is MsgDeployFungibleCoinZRC20, i.e. gas-coin onboarding for a new chain.

Probing the full x/fungible + x/crosschain suites shows exactly two accounts
reach the guard with a non-zero delta -- the x/fungible module account (131
burn, 1 mint) and the x/crosschain module account (13 burn). Neither is in
node's blockedReceivingModAcc set, which deliberately lists only the
invariant-bearing accounts: distribution, fee collector, bonded and not-bonded
pools, gov, evm, feemarket. So direction-based gating does not work either --
there is a legitimate mint -- but the chain's own blocked-receive policy draws
exactly the right line.

The guard now consults BankKeeper.BlockedAddr instead of testing for a module
account. Under the upstream evmd config every module account in maccPerms is
blocked, so behaviour there is unchanged and upstream's own guard tests pass
untouched; the divergence appears only on chains that deliberately allow a
module account to hold and move funds. This is the same check upstream already
uses for the analogous property in x/erc20/keeper/mint.go.

- x/vm/types: expose BlockedAddr on the BankKeeper interface (BankWrapper
  embeds it, so no pass-through is needed)
- x/vm/keeper: reject only blocked addresses, still only on a non-zero delta
- tests: replace the ad-hoc module-account arm, which is not blocked under evmd,
  with an explicit unblocked-module-account-moves-value arm carrying a premise
  assertion; let an accepted write land rather than asserting no-op either way

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Satisfies the changelog-check job, which is the only genuine CI failure on
#31 (test-solidity fails identically on the release/v35 base).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@kingpinXD kingpinXD left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
exact diff as the private pr

@skosito
skosito merged commit 51190f0 into release/v35 Sep 3, 2026
10 of 17 checks passed

@morde08 morde08 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving on equivalence to the private fork PR, which I verified mechanically rather than by reading the diffs.

Equivalence to evm-fork-2026-08-24#1

This branch is the private branch plus exactly one commit:

  • Commit SHAs are identical across the two repos (a8c94221b through feb2e93d3).
  • Base tips are identical across the two repos: release/v35 = f0addce1e5ac.
  • 21 files changed; for 20 of 21, both the pre- and post-image git blob hashes match the private PR. Blob hashes are content hashes, so those files are byte-identical.
  • The only content delta is CHANGELOG.md, one line: the private entry linked the superseded/closed #31, this one correctly self-links #36. Verified that 46cc608e7 touches nothing else.

Also confirmed the private #1 and #2 diffs are byte-identical to each other, which backs the claim that the v35 and v37 statedb files were identical at base.

Coverage against live node pins

  • node release/v36 (mainnet) pins evm f0addce1e5ac = base of #36
  • node release/zetacore/v37 (testnet) and node main pin 515bbca4d348 = base of #37

So both live lines plus main are covered. release/v33, release/v34, release/v37-0.4, release/v37-latest are pinned by nothing live.

Substance

Spot-checked the security-critical hunks and they carry our two deliberate divergences from upstream, not the raw upstream guard: SetBalanceWithLocked gates on bankWrapper.BlockedAddr (not "is a module account") and only when delta.Sign() != 0, which is what keeps x/fungible's gas-stability-pool BeginBlock burn and the ZRC20-deploy path alive. Commit() stages through a cache context; AddBalance/SubBalance panic via AddOverflow / balance.Lt(amount); onPacketTimeout now matches its three cachedCtx siblings.

One non-blocking observation: DeleteAccount now calls SetBalanceWithLocked(ctx, addr, 0, new(big.Int)), so the target is 0 + 0 and it burns spendable + locked, where the old SetBalance path burned only spendable. Harmless while no account carries locked coins, and arguably the more correct semantics for a deleted account, but it is a behavior change that isn't called out in the description.

Before merging

Please don't read the current board as CI clearance — test-unit-cover, test-fuzz, golangci-lint and Analyze (go) are still queued here at 0s, and they are the first genuine run these commits will ever get: the same four lanes on the private fork were CANCELLED after sitting ~24h without a runner, which gh pr checks renders as "fail". Worth waiting for test-unit-cover and golangci-lint to actually report before merge.

test-solidity red is unrelated and base-identical: Could not find a compiler version matching 0.8.18, a solc-bin fetch failure. check_diff passes here, unlike in the private fork.

skosito added a commit to zeta-chain/node that referenced this pull request Sep 3, 2026
…#4637)

fix(security): bump cosmos/evm to GHSA Aug-2026 patched commit (mainnet/v36)

Points the evm dependency at the patched release/v35 tip
(github.com/zeta-chain/evm @ 51190f04, zeta-chain/evm#36) carrying the
statedb underflow/overflow/atomicity guards, the module-account guard
gated on the chain's blocked-receive policy, and the x/ibc/callbacks
packet-timeout context fix.

Logic-only; no module-set/store change, no upgrade handler.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

4 participants