security(operator): invalidate reorged EVM deposits before dest approve #183
Labels
No labels
agent:implement
agent:ready
backend
bug
cannot-reproduce
confirmed
desktop
docs
documentation
duplicate
enhancement
feature
frontend
good first issue
help wanted
high-risk
in-review
invalid
mobile
needs-triage
P0-critical
P1-high
P2-medium
P3-low
qa
QA
question
ready
report
responsive
security
security-escalate
smart-contract
solana
tablet
test-pass
ux
wallet-issue
wallet:keplr
wallet:metamask
wallet:station
wallet:walletconnect
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-bridge-monorepo#183
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
The operator EVM deposit watcher stores
evm_deposits.block_hashon every ingest and subtractsfinality_blocksfrom head beforeeth_getLogs. That is ingest delay only. Nothing later re-checks that the stored height still has the stored hash, nothing marks a depositreorged, and destWithdrawApprovecan still treat the row as actionable.update_approval_reorgedandupdate_release_reorgedexist inpackages/operator/src/db/mod.rsand have no callers. There is no deposit-status equivalent. After a source reorg, pending rows staypendingand dest-side approval/release rows never move toreorged.This is not #182 (RS-H1: Terra watcher has no confirmation-depth buffer). This is not closed #115 (EVM-02 tuned
FINALITY_BLOCKSdefaults; it treated the mechanic as already present and did not wire inclusion revalidation). This is not #138 (writer livelock / retry).Internal review id: RS-H2 (high). Still in source as of 2026-09-12 (
packages/operator/src/db/mod.rs+watchers/evm.rs+writers/evm.rsonmain).Bundle (same ticket, do not split):
evm_deposits: canonical block hash atblock_numbermust still equal storedblock_hash, and the tx/log (orgetDeposit(hash)at that block tag) must still be present.reorged(or equivalent non-pending status) when inclusion fails. Wireupdate_approval_reorged/update_release_reorgedfor dest-side rows that were submitted against a now-orphaned source (or add a deposit-level helper and call it). Do not leave the source rowpending.WithdrawApproveuntil source finality and inclusion. Do not approve from a DB row, a tipgetDeposit, or a height whose hash no longer matches.Founder-required operator dest-approve / source ingest. No community autoland. Do not add
ready.Impact (today vs hypothetical)
Funds at risk today on any live operator that dest-approves EVM-origin withdrawals from a stored
evm_depositsrow (or fromgetDepositat latest) without re-checking the storedblock_hash. Confirmation depth (FINALITY_BLOCKS: BSC 15, opBNB 12) lowers the odds of a shallow reorg during ingest. It does not invalidate a row whose height later points at a different hash, and it does not stop dest approve if the source contract still returns a tip-state deposit that the canonical chain later drops.This is not permissionless theft of unrelated vault inventory: the attacker (or a colliding honest user after a halt/reorg) needs an EVM deposit the operator observed inside the confirmation window. Severity is high because dest mint/unlock is the payout. Rate limits and dest cancel windows are not a substitute for source inclusion + finality.
Hypothetical-only if every dest-approve path already compared stored
block_hashto canonicaleth_getBlockByNumber(height)and refused on mismatch (they do not: the column is write-only) or ifgetDepositwere always queried at a finalized block tag with a hash check (it uses latest HTTP RPC). Sticky until inclusion re-check, status update, and dest-approve gate land.Do not publish a mainnet deposit-then-reorg-then-dest-approve sequence.
block_hashis stored and never read backEvmWatcherparse paths requirelog.block_hashand persist it viainsert_evm_deposit.get_pending_evm_deposits*SELECTsblock_hashontoEvmDeposit. No watcher/writer compares that field to canonical chain state.evm_deposit_existskeys on(chain_id, tx_hash, log_index)only, so a replacement log at the same height does not update or invalidate the old row.Confirmation depth is ingest-only
EvmWatcher::get_finalized_blockislatest.saturating_sub(finality_blocks). Closed #115 EVM-02 raised the default. After insert,lastcursor advances; there is no lookback that re-fetches those heights and compares hashes. A reorg deeper than N, or a hash mismatch at a height already behind finalized head, leaves the rowpending.Dest approve does not bind inclusion
EvmWriter::enumerate_and_approve→verify_deposit_on_source→verify_evm_deposit_on_chaincallsgetDeposit(hash)on latest RPC. Non-zero timestamp + matchingdestChainis enough. There is noblock_hashcompare and no finalized block tag.The leftover DB path
process_evm_to_evm_pending→process_evm_depositbuildsxchain_hash_idfrom the stored row,insert_approval, andsubmit_evm_to_evm_approvalwithout an inclusion re-check.update_evm_deposit_status(..., "approved")/"processed"never uses"reorged".Reorg helpers are dead code
update_approval_reorged/update_release_reorgedSETapprovals/releasesto'reorged'. Forgejo code search shows only those definitions. Dest-side submitted txs that vanish after a dest reorg also staysubmitted/ retryable. Deposit rows haveupdate_evm_deposit_statusbut no caller passesreorged.Invariants
evm_depositsrow is dest-approvable only if canonical hash atblock_numberequals storedblock_hashand the deposit is still included (tx/log orgetDepositat that block / finalized tag).pending(reorged/ invalid).get_pending_evm_deposits*must not return it.WithdrawApprovemust fail closed on inclusion failure, RPC error, or hash mismatch. A DB row is not proof.approvals/releaseswhose submitted tx is absent from the canonical dest chain must move toreorged(wire the existing helpers or replace them with one tested path). Do not retry as if the tx were still in flight.finality_blocks, RPC quorum, dest cancel window, userwithdrawSubmit, or hash inputs. Do not skip dest verify solely because a DB row exists.Constraints / guardrails
block_hash+block_number; do not invent a second cursor. Prefereth_getBlockByNumber(block_number)(quorum / consensus endpoint, same as watcher) vs stored hash; then confirm the tx is still in that block (receiptblockHashmatch, orgetDepositat that block tag).finality_blocksstays the ingest poll end (#115). This ticket adds post-ingest revalidation + dest-approve gate, not a new default N.ready. No public mainnet reorg recipe.Relevant files
packages/operator/src/db/mod.rsblock_hashinsert/select; deadupdate_approval_reorged/update_release_reorged;update_evm_deposit_status; pending queriespackages/operator/src/watchers/evm.rsblock_hash;get_finalized_block; insert-once, no hash revalidationpackages/operator/src/writers/evm.rsverify_evm_deposit_on_chain(latestgetDeposit);process_evm_depositdest-approves from DBpackages/operator/src/writers/terra.rsRecommended direction
update_evm_deposit_reorged(or callupdate_evm_deposit_status(..., "reorged")from one helper). Pending SELECTs staystatus = 'pending'only.block_number; on mismatch or missing tx/log, mark reorged; do not dest-approve.verify_evm_deposit_on_chainandprocess_evm_deposit): require inclusion + finalized (deposit height + N ≤ current finalized head, orgetDepositat finalized block tag). Hash mismatch → reorged + skip.update_approval_reorged/update_release_reorgedfrom dest receipt/confirmation: if a submitted dest tx’s receipt is missing orblockHashdiverged, mark reorged instead of retry-as-submitted.Acceptance criteria
block_hash≠ canonical hash atblock_numberis not returned byget_pending_evm_deposits*and is not dest-approved.reorged(or documented equivalent) and dest approve is refused.WithdrawApproveis not submitted until source inclusion holds at finalized depth (ingest N plus hash check). TipgetDepositalone is not enough.update_approval_reorged/update_release_reorged(or the replacement helper) have a production caller; dest submitted txs that vanish are not retried as livesubmitted.FINALITY_BLOCKS/get_finalized_blockingest behavior unchanged. Terra confirmation depth stays #182.Test plan (functional paths)
reorged; not pending; dest approve falsegetDepositlatest true, hash mismatchprocess_evm_depositwith mismatched hashwithdrawApprove; noapprovedapprovals.status = reorged; not retried as submittedfinality_blocksTest plan (attack, hack, and abuse)
Non-exploitative. Local RPC mocks / operator unit tests only. Do not use these as a mainnet recipe.
getDepositstill returns the old hash at latest while canonical block hash divergedgetDeposittimestamp 0reorged; not treated as confirmedVerification criteria
block_hashis compared on the dest-approve / pending-revalidation path (not only INSERT). Grep a production call toupdate_approval_reorged/update_release_reorgedor the replacement helper (not only thedb/mod.rsdefinition).get_pending_evm_depositscannot return a row whose stored hash failed the last inclusion check.Out of scope
TerraConfig.finality_blocks./health,eth_chainId, raising EVM default N.getDepositlayout / hash words.First-pass model recommendation
Recommendation: grok-high
Rationale: Security class and founder-required operator dest-approve / source ingest (wallet / bridge funds). Composer is disallowed (High/security; not a low-risk first pass). Scope is not a local three-file tweak: pending inclusion re-check, deposit status, dest-approve gates in
writers/evm.rs(and Terra dest for EVM-source), wiring dead approval/release reorg helpers, and new tests. A wrong allow (dest-approve after source hash divergence) can mint dest assets for a lock that is not on the canonical chain. Verify with mocked block-hash / missing-tx fixtures, not a live chain reorg.