fix(security): port cosmos/evm statedb balance hardening to the v36 line - #31
Open
morde08 wants to merge 4 commits into
Open
fix(security): port cosmos/evm statedb balance hardening to the v36 line#31morde08 wants to merge 4 commits into
morde08 wants to merge 4 commits into
Conversation
…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>
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.
Summary
Ports the three
cosmos/evmbalance-hardening fixes onto the branch that ZetaChain v36 (current mainnet) pins, plus the adaptations needed to make them correct and non-breaking on this fork.Base is
release/v35, whose tipf0addce1is exactly whatnode'srelease/v36pins ingo.mod. (The v37 line pinsrelease/v37@515bbca4— a companion PR will be needed there.)Context — we are not exposed to the live exploit
The exploit is now public:
pushchain/push-chain-evm#40(Hacken F-2026-18201), and MANTRA confirmed the staking precompile as the vector.The real trigger is a vesting account delegating locked coins through the staking precompile: spendable
S< delegatedD, the balance handler mirrorscoin_spent(D),SubBalancewraps to ~2^256, andSetBalancereconciliation mints (or drains a victim).ZetaChain blocks that precondition already.
app/ante/vesting.gorejectsMsgCreateVestingAccount,MsgCreatePermanentLockedAccountandMsgCreatePeriodicVestingAccountwithErrUnauthorized, wired into all three ante chains (app/ante/handler_options.go:43,74,103), present inrelease/v36, and added in cosmos#923 on 2023-08-11 — before mainnet launch, for an unrelated reason (contract address bricking). In cosmos-sdk, vesting accounts are the only source of non-zeroLockedCoins, and we add no custom account types. That is the same mitigation Push is adding in their companion PR cosmos#315.Two secondary reasons the other halves are inert for us:
ParseAmountdenom handling —cmd/zetacored/root.go:175-179setsDenom == ExtendedDenomatDecimals: 18, the 18-decimal conversion factor is1e0(identity), andprecisebankis not wired intoapp/app.go. This is the half that hit MANTRA (6-decimal OM, factor1e12).blockedReceivingModAcc(app/app.go:186-194) already blocks the bonded/not-bonded pools, distribution, gov, evm and feemarket from receiving.So this is defense-in-depth, not an emergency. It should ride the normal release train rather than force an out-of-band upgrade. Note that the safety argument rests on the ante-handler block — if vesting-account creation is ever re-enabled, or a locked-coin account type is added, we become exposed and this patch becomes load-bearing.
What's included
3524ebcSubBalanceunderflow panic + denom-awareParseAmountOur fork descends from
v1.0.0-rc2(June 2025);v0.6.2/v0.7.2are Aug 2026 tags on divergent maintenance branches, so there is no clean upgrade path — these had to be ported. Unrelated hunks bundled into the upstream PRs (README.md,tests/soliditydependency bumps) are deliberately not taken.Three adaptations (final commit)
1. The
ParseAmountfix landed on dead code. This fork carries duplicate unexportedparseAmount/parseHexAddressinbalance_handler.go, while cosmos#1253 patches only the exportedParseAmountinutils.go. The live path was left unpatched by the cherry-pick — a silent no-op. Deduplicated onto the patched exported helpers, converging with upstream.2.
utils.gowould not compile. The patchedParseAmountbody usesbig.Int, but the import hunk did not apply against our import block. Added"math/big".3. The module-account guard had to be narrowed — please review this closely. Upstream rejects module accounts unconditionally at the top of
SetBalanceWithLocked, before computing the delta. That breaks any EVM call made from a module account, andx/fungibleissues those pervasively — ZRC20 deploys, gas-pool swaps, system-contract calls (x/fungible/keeper/evm.go:96,205,258,289,313,337). Each makes the module account dirty in the statedb and reachesSetBalancewithdelta == 0, so the unconditional guard would brick the omnichain core. This was caught byTestCallEVMWithData/Case_deployfailing only with the patch applied.The guard now fires only when
delta != 0. A zero delta mints and burns nothing, so the security property is identical — no mint or burn can be reconciled into a module account — while module-initiated EVM calls keep working. The divergence is commented at the call site, andTestSetBalanceRejectsModuleAccountsgains an explicit mint-direction case (the exploit direction), which upstream did not cover.Verification
New upstream security tests all pass, including
TestSetBalanceRejectsModuleAccounts(all four arms),TestSetBalanceWithLocked,TestGetAccountLockedandTestSubBalanceUnderflowPanics.Pre-existing failures, unchanged:
TestCallEVMWithData/{Case_pass_with_empty_data,Case_pass_with_unknown_method}fail identically on the unpatched base commitf0addce1— verified by running the fullTestKeeperTestSuiteon both and diffing the failure sets, which are identical.go vet ./...also reports pre-existingunreachable codein generated*.pulsar.go.Follow-ups
release/v37for the v37 line (515bbca4).cosmos/evmreplace directive innode'sgo.modonce this merges — the patch does nothing until the pin moves.🤖 Generated with Claude Code