Skip to content

fix(security): port GHSA Aug-2026 statedb hardening (v37/testnet) - #37

Merged
skosito merged 4 commits into
release/v37from
sec/testnet-v37-complete
Sep 3, 2026
Merged

skosito merged 4 commits into
release/v37from
sec/testnet-v37-complete

Conversation

@skosito

@skosito skosito commented Sep 3, 2026 •

Copy link
Copy Markdown
Member

Ports the complete balance-hardening set to the fork line that node release/zetacore/v37 (testnet) pins. The v37 line had none of the fixes; the statedb files were byte-identical to the v35 base, so this is the same change set as PR #1.

Changes (full set)

  • SubBalance underflow guard + ParseAmount extended-denom handling
  • AddBalance overflow guard
  • StateDB.Commit() atomicity (cache-context staging)
  • Module-account guard gated on delta.Sign() != 0
  • Snapshot locked balance on statedb account
  • x/ibc/callbacks onPacketTimeout: cachedCtx, not live ctx
  • TestCommitAtomicity regression test

CI note

Run locally: go test ./x/vm/statedb/... -tags test -count=1 (green; atomicity test mutation-verified). Aggregate CI has unrelated pre-existing red (check_diff, test-solidity).

Ship/validate on testnet first, then promote the equivalent v35 change to mainnet.


Note

High Risk
Changes core EVM/bank reconciliation, commit atomicity, and fund-blocking rules on security-sensitive paths; incorrect behavior could affect balances, vesting, or module accounts.

Overview
Ports upstream cosmos/evm StateDB balance hardening to the v37 fork: accounts now carry a locked-balance snapshot at load time, AddBalance/SubBalance panic on overflow/underflow, and StateDB.Commit() stages writes through a cache context so a late failure rolls back the whole commit (including precompile-staged writes).

Bank reconciliation on commit is reworked via SetBalanceWithLocked / SetAccountBalance, reconstructing bank totals as spendable + locked. Mint/burn into blocked receive addresses is rejected only when delta ≠ 0, using the chain’s BlockedAddr policy instead of rejecting every module account—so allowed module accounts (e.g. fungible gas pool) can still reconcile while staking pools stay protected.

Precompile bank-event sync now uses exported ParseHexAddress / ParseAmount, with ParseAmount summing base (18-decimal scaled) and extended EVM coin denoms. x/ibc/callbacks packet-timeout EVM calls use cachedCtx. x/erc20 ERC20 registration reuses the existing account when updating code hash. x/precisebank exposes LockedCoins for the VM keeper. Tests cover atomic commit, vesting delegation, blocked-pool transfers, and reconciliation edge cases.

Reviewed by Cursor Bugbot for commit 6c1c110. Configure here.

morde08 and others added 3 commits August 24, 2026 23:11
Ports the complete balance-hardening set to the release/v37 fork line
(node release/zetacore/v37 / testnet), matching the v35/mainnet branch:
- SubBalance underflow guard + ParseAmount extended-denom handling
- AddBalance overflow guard
- StateDB.Commit() atomicity (cache-context staging)
- module-account guard gated on non-zero delta (keeps x/fungible working)
- snapshot locked balance on statedb account
- x/ibc/callbacks onPacketTimeout: cachedCtx, not live ctx
- TestCommitAtomicity regression test

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

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>
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 a69d910 into release/v37 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#2

This branch is the private branch plus exactly one commit:

  • Commit SHAs are identical across the two repos (198da04bf through 6c1c110ad).
  • Base tips are identical across the two repos: release/v37 = 515bbca4d348.
  • 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 #37. Verified that f2aae11a5 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
…#4638)

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

Points the evm dependency at the patched release/v37 tip
(github.com/zeta-chain/evm @ a69d910e, zeta-chain/evm#37).

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.

3 participants