Skip to content

fix(contracts): bind cross-vault state in lifecycle authorization - #133

Merged
mellowcroc merged 7 commits into
mainfrom
fix/f-2026-19300-parent-state-binding
Sep 15, 2026
Merged

mellowcroc merged 7 commits into
mainfrom
fix/f-2026-19300-parent-state-binding

Conversation

@mellowcroc

@mellowcroc mellowcroc commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • explicitly attach foreign RootVault and SubVault AccountUpdate objects before reading cross-vault state
  • bind RootVault approval state during all four child lifecycle executions
  • bind the nonce-authority SubVault state during parent propose() and approveProposal()
  • require each foreign update to be a direct child of the proof-authorized update
  • add malicious-prover regressions and genuine-proof coverage for both cross-vault directions
  • document the call-forest and precondition requirements

Security impact

Fixes F-2026-19300. Previously, parent state reads were recorded on an unattached AccountUpdate, allowing a prover to substitute a forged approval root and threshold for CREATE_CHILD, RECLAIM_CHILD, DESTROY_CHILD, and ENABLE_CHILD_MULTI_SIG. Attaching the RootVault update makes Mina compare those values with the actual RootVault account state.

The same detached-read pattern also existed in assertFreshProposalNonce(): parent propose() and approveProposal() could prove against substituted child ownersCommitment, parent, and parentNonce values. This was not an independent unauthorized-execution path because the child rechecks its real state during execution, but the proposal/approval circuit was not enforcing the child-state validation it claimed to enforce. This PR now attaches that nonce-authority update as well.

Both foreign updates are precondition-only: they do not mutate the referenced vault or require its owner signature.

Regression coverage

The fast malicious-prover tests assert that:

  • all four affected child lifecycle execution methods reject forged RootVault state
  • both propose() and approveProposal() reject forged SubVault state
  • exactly one constrained foreign-vault update is serialized
  • the foreign update is a direct child of the proof update, so the proof commits to it
  • the expected state slots are constrained
  • ledger application fails specifically with Account_app_state_precondition_unsatisfied

Two opt-in proof-enabled tests repeat the highest-impact forged-parent executeDestroy attack and the forged-child propose() attack. Each asserts that o1js produced a real proof object rather than a dummy proof before confirming rejection.

Verification keys

The circuit changed, so both canonical hashes were regenerated after synchronizing with current main:

  • testnet: 24549654168200894274343469105778304028445635856968658698145262137123892530854
  • mainnet: 23011579427028232763944993528190930908391338988194510399893639369434906880365

Testing

  • bun run --filter contracts build
  • bun run --filter contracts typecheck
  • bun run --filter contracts test — 152 passed, 0 failed, 3 intentional skips before the final main sync
  • forged child-state propose() and approveProposal() regressions after sync — 2 passed
  • LOCAL propose/approve suite — 28 passed
  • atomic CREATE_CHILD setup regression — passed
  • RUN_REAL_PROOF_TESTS=1 bun test src/tests/child.test.ts --test-name-pattern 'genuine MinaGuard proof' --timeout 900000 after sync — 2 passed; real proofs produced and rejected for the expected foreign-state precondition mismatches
  • clean-cache testnet and mainnet VK compilation after sync completed with the hashes above
  • VK hash helper now forces recompilation so stale cache artifacts cannot be accepted

@mellowcroc
mellowcroc marked this pull request as ready for review September 14, 2026 14:05
Comment thread contracts/src/MinaGuard.ts
@mellowcroc mellowcroc changed the title fix(contracts): bind RootVault state in child lifecycle proofs fix(contracts): bind cross-vault state in lifecycle authorization Sep 15, 2026
…nt-state-binding

# Conflicts:
#	contracts/.vk-hash
#	contracts/src/tests/child.test.ts
@mellowcroc
mellowcroc requested a review from graikos September 15, 2026 08:39
@mellowcroc
mellowcroc merged commit 221df8c into main Sep 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants