security(operator): invalidate reorged EVM deposits before dest approve #183

Open
opened 2026-09-12 12:42:39 +00:00 by PlasticDigits · 0 comments

Summary

The operator EVM deposit watcher stores evm_deposits.block_hash on every ingest and subtracts finality_blocks from head before eth_getLogs. That is ingest delay only. Nothing later re-checks that the stored height still has the stored hash, nothing marks a deposit reorged, and dest WithdrawApprove can still treat the row as actionable.

update_approval_reorged and update_release_reorged exist in packages/operator/src/db/mod.rs and have no callers. There is no deposit-status equivalent. After a source reorg, pending rows stay pending and dest-side approval/release rows never move to reorged.

This is not #182 (RS-H1: Terra watcher has no confirmation-depth buffer). This is not closed #115 (EVM-02 tuned FINALITY_BLOCKS defaults; 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.rs on main).

Bundle (same ticket, do not split):

  1. Re-check inclusion for pending evm_deposits: canonical block hash at block_number must still equal stored block_hash, and the tx/log (or getDeposit(hash) at that block tag) must still be present.
  2. Mark the deposit reorged (or equivalent non-pending status) when inclusion fails. Wire update_approval_reorged / update_release_reorged for 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 row pending.
  3. Block dest WithdrawApprove until source finality and inclusion. Do not approve from a DB row, a tip getDeposit, or a height whose hash no longer matches.
  4. Tests that a reorged source block cannot stay actionable (pending query + dest approve both fail closed).

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_deposits row (or from getDeposit at latest) without re-checking the stored block_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_hash to canonical eth_getBlockByNumber(height) and refused on mismatch (they do not: the column is write-only) or if getDeposit were 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_hash is stored and never read back

EvmWatcher parse paths require log.block_hash and persist it via insert_evm_deposit. get_pending_evm_deposits* SELECTs block_hash onto EvmDeposit. No watcher/writer compares that field to canonical chain state. evm_deposit_exists keys 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_block is latest.saturating_sub(finality_blocks). Closed #115 EVM-02 raised the default. After insert, last cursor 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 row pending.

Dest approve does not bind inclusion

EvmWriter::enumerate_and_approve → verify_deposit_on_source → verify_evm_deposit_on_chain calls getDeposit(hash) on latest RPC. Non-zero timestamp + matching destChain is enough. There is no block_hash compare and no finalized block tag.

The leftover DB path process_evm_to_evm_pending → process_evm_deposit builds xchain_hash_id from the stored row, insert_approval, and submit_evm_to_evm_approval without an inclusion re-check. update_evm_deposit_status(..., "approved") / "processed" never uses "reorged".

Reorg helpers are dead code

update_approval_reorged / update_release_reorged SET approvals / releases to 'reorged'. Forgejo code search shows only those definitions. Dest-side submitted txs that vanish after a dest reorg also stay submitted / retryable. Deposit rows have update_evm_deposit_status but no caller passes reorged.

Invariants

  • INV-OP-E1 (new): A pending evm_deposits row is dest-approvable only if canonical hash at block_number equals stored block_hash and the deposit is still included (tx/log or getDeposit at that block / finalized tag).
  • INV-OP-E2 (new): When inclusion fails, the deposit must leave pending (reorged / invalid). get_pending_evm_deposits* must not return it.
  • INV-OP-E3 (new): Dest WithdrawApprove must fail closed on inclusion failure, RPC error, or hash mismatch. A DB row is not proof.
  • INV-OP-E4 (new): Dest-side approvals / releases whose submitted tx is absent from the canonical dest chain must move to reorged (wire the existing helpers or replace them with one tested path). Do not retry as if the tx were still in flight.
  • Do not weaken EVM finality_blocks, RPC quorum, dest cancel window, user withdrawSubmit, or hash inputs. Do not skip dest verify solely because a DB row exists.

Constraints / guardrails

  • Reuse stored block_hash + block_number; do not invent a second cursor. Prefer eth_getBlockByNumber(block_number) (quorum / consensus endpoint, same as watcher) vs stored hash; then confirm the tx is still in that block (receipt blockHash match, or getDeposit at that block tag).
  • Fail closed on RPC errors. Do not dest-approve when the hash RPC fails.
  • Do not rewind the global EVM watcher cursor on every hash blip; invalidate rows whose height’s canonical hash diverged. Optional short lookback behind finalized head is in scope if tested.
  • finality_blocks stays the ingest poll end (#115). This ticket adds post-ingest revalidation + dest-approve gate, not a new default N.
  • Do not change Solidity / CosmWasm / Solana programs. Do not retune Terra confirmation depth here (#182).
  • Founder-required operator / dest approve. No community autoland. Do not add ready. No public mainnet reorg recipe.

Relevant files

Path Why
packages/operator/src/db/mod.rs block_hash insert/select; dead update_approval_reorged / update_release_reorged; update_evm_deposit_status; pending queries
packages/operator/src/watchers/evm.rs Writes block_hash; get_finalized_block; insert-once, no hash revalidation
packages/operator/src/writers/evm.rs verify_evm_deposit_on_chain (latest getDeposit); process_evm_deposit dest-approves from DB
packages/operator/src/writers/terra.rs EVM→Terra dest approve must use the same inclusion gate
Operator DB/watcher/writer tests Add hash-mismatch / missing-tx fixtures
  1. Add update_evm_deposit_reorged (or call update_evm_deposit_status(..., "reorged") from one helper). Pending SELECTs stay status = 'pending' only.
  2. Watcher or writer pass over pending EVM deposits: fetch canonical hash at block_number; on mismatch or missing tx/log, mark reorged; do not dest-approve.
  3. Dest-approve gates (verify_evm_deposit_on_chain and process_evm_deposit): require inclusion + finalized (deposit height + N ≤ current finalized head, or getDeposit at finalized block tag). Hash mismatch → reorged + skip.
  4. Wire update_approval_reorged / update_release_reorged from dest receipt/confirmation: if a submitted dest tx’s receipt is missing or blockHash diverged, mark reorged instead of retry-as-submitted.
  5. Tests: stored hash ≠ canonical hash → not in pending, dest approve false. Missing tx at that height → same. Honest matching hash + N-deep still approves.

Acceptance criteria

  • AC1. Pending EVM deposit whose stored block_hash ≠ canonical hash at block_number is not returned by get_pending_evm_deposits* and is not dest-approved.
  • AC2. Pending EVM deposit whose tx/log is absent at that height (reorg fixture) is marked reorged (or documented equivalent) and dest approve is refused.
  • AC3. Dest WithdrawApprove is not submitted until source inclusion holds at finalized depth (ingest N plus hash check). Tip getDeposit alone is not enough.
  • AC4. update_approval_reorged / update_release_reorged (or the replacement helper) have a production caller; dest submitted txs that vanish are not retried as live submitted.
  • AC5. EVM FINALITY_BLOCKS / get_finalized_block ingest behavior unchanged. Terra confirmation depth stays #182.
  • AC6. Existing operator tests stay green. New unit tests cover AC1–AC3 without a live BSC/opBNB node.

Test plan (functional paths)

# Path Expect
T1 Insert deposit at height H with hash A; canonical H is still A; N-deep Still pending; dest approve allowed after finality
T2 Same row; canonical H hash is B Status reorged; not pending; dest approve false
T3 Canonical hash A but tx missing from block H Same as T2
T4 getDeposit latest true, hash mismatch Dest approve false; row reorged
T5 process_evm_deposit with mismatched hash No withdrawApprove; no approved
T6 RPC hash lookup error Fail closed; no approve; no cursor rewind
T7 Dest approval receipt missing / hash changed approvals.status = reorged; not retried as submitted
T8 Honest EVM watcher fixture with finality_blocks Ingest window unchanged

Test plan (attack, hack, and abuse)

Non-exploitative. Local RPC mocks / operator unit tests only. Do not use these as a mainnet recipe.

# Vector Expect
A1 Deposit ingested, then source block hash replaced No dest approve; row not pending
A2 Replacement tx at same height, different hash/nonce Old row reorged; new tx only after ingest + inclusion
A3 getDeposit still returns the old hash at latest while canonical block hash diverged Fail closed on hash check
A4 DB row present, getDeposit timestamp 0 Fail closed (today); keep it; also mark reorged
A5 Dest approve submitted, dest chain reorgs the approve tx reorged; not treated as confirmed

Verification criteria

  • Operator package tests: T1–T8 and A1–A5. Grep that block_hash is compared on the dest-approve / pending-revalidation path (not only INSERT). Grep a production call to update_approval_reorged / update_release_reorged or the replacement helper (not only the db/mod.rs definition).
  • get_pending_evm_deposits cannot return a row whose stored hash failed the last inclusion check.
  • Do not verify by forcing a BSC/opBNB reorg or by dest-approving a reorged deposit on production.

Out of scope

  • #182 Terra LCD latest / TerraConfig.finality_blocks.
  • #115 RPC quorum, HTTPS, /health, eth_chainId, raising EVM default N.
  • #138 writer livelock.
  • #170 dest-approved Terra→EVM execute stall.
  • Solidity getDeposit layout / hash words.
  • Live operator redeploy / RPC URL changes (ops).

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.

## Summary The operator EVM deposit watcher stores `evm_deposits.block_hash` on every ingest and subtracts `finality_blocks` from head before `eth_getLogs`. That is ingest delay only. Nothing later re-checks that the stored height still has the stored hash, nothing marks a deposit `reorged`, and dest `WithdrawApprove` can still treat the row as actionable. `update_approval_reorged` and `update_release_reorged` exist in `packages/operator/src/db/mod.rs` and have no callers. There is no deposit-status equivalent. After a source reorg, pending rows stay `pending` and dest-side approval/release rows never move to `reorged`. This is not [#182](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/182) (RS-H1: Terra watcher has **no** confirmation-depth buffer). This is not closed [#115](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/115) (EVM-02 tuned `FINALITY_BLOCKS` defaults; it treated the mechanic as already present and did not wire inclusion revalidation). This is not [#138](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/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.rs` on `main`). Bundle (same ticket, do not split): 1. Re-check inclusion for pending `evm_deposits`: canonical block hash at `block_number` must still equal stored `block_hash`, and the tx/log (or `getDeposit(hash)` at that block tag) must still be present. 2. Mark the deposit `reorged` (or equivalent non-pending status) when inclusion fails. Wire `update_approval_reorged` / `update_release_reorged` for 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 row `pending`. 3. Block dest `WithdrawApprove` until source finality **and** inclusion. Do not approve from a DB row, a tip `getDeposit`, or a height whose hash no longer matches. 4. Tests that a reorged source block cannot stay actionable (pending query + dest approve both fail closed). 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_deposits` row (or from `getDeposit` at latest) without re-checking the stored `block_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_hash` to canonical `eth_getBlockByNumber(height)` and refused on mismatch (they do not: the column is write-only) or if `getDeposit` were 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_hash` is stored and never read back `EvmWatcher` parse paths require `log.block_hash` and persist it via `insert_evm_deposit`. `get_pending_evm_deposits*` SELECTs `block_hash` onto `EvmDeposit`. No watcher/writer compares that field to canonical chain state. `evm_deposit_exists` keys 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_block` is `latest.saturating_sub(finality_blocks)`. Closed #115 EVM-02 raised the default. After insert, `last` cursor 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 row `pending`. ### Dest approve does not bind inclusion `EvmWriter::enumerate_and_approve` → `verify_deposit_on_source` → `verify_evm_deposit_on_chain` calls `getDeposit(hash)` on latest RPC. Non-zero timestamp + matching `destChain` is enough. There is no `block_hash` compare and no finalized block tag. The leftover DB path `process_evm_to_evm_pending` → `process_evm_deposit` builds `xchain_hash_id` from the stored row, `insert_approval`, and `submit_evm_to_evm_approval` without an inclusion re-check. `update_evm_deposit_status(..., "approved")` / `"processed"` never uses `"reorged"`. ### Reorg helpers are dead code `update_approval_reorged` / `update_release_reorged` SET `approvals` / `releases` to `'reorged'`. Forgejo code search shows only those definitions. Dest-side submitted txs that vanish after a dest reorg also stay `submitted` / retryable. Deposit rows have `update_evm_deposit_status` but no caller passes `reorged`. ## Invariants - INV-OP-E1 (new): A pending `evm_deposits` row is dest-approvable only if canonical hash at `block_number` equals stored `block_hash` **and** the deposit is still included (tx/log or `getDeposit` at that block / finalized tag). - INV-OP-E2 (new): When inclusion fails, the deposit must leave `pending` (`reorged` / invalid). `get_pending_evm_deposits*` must not return it. - INV-OP-E3 (new): Dest `WithdrawApprove` must fail closed on inclusion failure, RPC error, or hash mismatch. A DB row is not proof. - INV-OP-E4 (new): Dest-side `approvals` / `releases` whose submitted tx is absent from the canonical dest chain must move to `reorged` (wire the existing helpers or replace them with one tested path). Do not retry as if the tx were still in flight. - Do not weaken EVM `finality_blocks`, RPC quorum, dest cancel window, user `withdrawSubmit`, or hash inputs. Do not skip dest verify solely because a DB row exists. ## Constraints / guardrails - Reuse stored `block_hash` + `block_number`; do not invent a second cursor. Prefer `eth_getBlockByNumber(block_number)` (quorum / consensus endpoint, same as watcher) vs stored hash; then confirm the tx is still in that block (receipt `blockHash` match, or `getDeposit` at that block tag). - Fail closed on RPC errors. Do not dest-approve when the hash RPC fails. - Do not rewind the global EVM watcher cursor on every hash blip; invalidate **rows** whose height’s canonical hash diverged. Optional short lookback behind finalized head is in scope if tested. - `finality_blocks` stays the ingest poll end (#115). This ticket adds post-ingest revalidation + dest-approve gate, not a new default N. - Do not change Solidity / CosmWasm / Solana programs. Do not retune Terra confirmation depth here (#182). - Founder-required operator / dest approve. No community autoland. Do not add `ready`. No public mainnet reorg recipe. ## Relevant files | Path | Why | | --- | --- | | `packages/operator/src/db/mod.rs` | `block_hash` insert/select; dead `update_approval_reorged` / `update_release_reorged`; `update_evm_deposit_status`; pending queries | | `packages/operator/src/watchers/evm.rs` | Writes `block_hash`; `get_finalized_block`; insert-once, no hash revalidation | | `packages/operator/src/writers/evm.rs` | `verify_evm_deposit_on_chain` (latest `getDeposit`); `process_evm_deposit` dest-approves from DB | | `packages/operator/src/writers/terra.rs` | EVM→Terra dest approve must use the same inclusion gate | | Operator DB/watcher/writer tests | Add hash-mismatch / missing-tx fixtures | ## Recommended direction 1. Add `update_evm_deposit_reorged` (or call `update_evm_deposit_status(..., "reorged")` from one helper). Pending SELECTs stay `status = 'pending'` only. 2. Watcher or writer pass over pending EVM deposits: fetch canonical hash at `block_number`; on mismatch or missing tx/log, mark reorged; do not dest-approve. 3. Dest-approve gates (`verify_evm_deposit_on_chain` and `process_evm_deposit`): require inclusion + finalized (deposit height + N ≤ current finalized head, or `getDeposit` at finalized block tag). Hash mismatch → reorged + skip. 4. Wire `update_approval_reorged` / `update_release_reorged` from dest receipt/confirmation: if a submitted dest tx’s receipt is missing or `blockHash` diverged, mark reorged instead of retry-as-submitted. 5. Tests: stored hash ≠ canonical hash → not in pending, dest approve false. Missing tx at that height → same. Honest matching hash + N-deep still approves. ## Acceptance criteria - AC1. Pending EVM deposit whose stored `block_hash` ≠ canonical hash at `block_number` is not returned by `get_pending_evm_deposits*` and is not dest-approved. - AC2. Pending EVM deposit whose tx/log is absent at that height (reorg fixture) is marked `reorged` (or documented equivalent) and dest approve is refused. - AC3. Dest `WithdrawApprove` is not submitted until source inclusion holds at finalized depth (ingest N plus hash check). Tip `getDeposit` alone is not enough. - AC4. `update_approval_reorged` / `update_release_reorged` (or the replacement helper) have a production caller; dest submitted txs that vanish are not retried as live `submitted`. - AC5. EVM `FINALITY_BLOCKS` / `get_finalized_block` ingest behavior unchanged. Terra confirmation depth stays #182. - AC6. Existing operator tests stay green. New unit tests cover AC1–AC3 without a live BSC/opBNB node. ## Test plan (functional paths) | # | Path | Expect | | --- | --- | --- | | T1 | Insert deposit at height H with hash A; canonical H is still A; N-deep | Still pending; dest approve allowed after finality | | T2 | Same row; canonical H hash is B | Status `reorged`; not pending; dest approve false | | T3 | Canonical hash A but tx missing from block H | Same as T2 | | T4 | `getDeposit` latest true, hash mismatch | Dest approve false; row reorged | | T5 | `process_evm_deposit` with mismatched hash | No `withdrawApprove`; no `approved` | | T6 | RPC hash lookup error | Fail closed; no approve; no cursor rewind | | T7 | Dest approval receipt missing / hash changed | `approvals.status = reorged`; not retried as submitted | | T8 | Honest EVM watcher fixture with `finality_blocks` | Ingest window unchanged | ## Test plan (attack, hack, and abuse) Non-exploitative. Local RPC mocks / operator unit tests only. Do not use these as a mainnet recipe. | # | Vector | Expect | | --- | --- | --- | | A1 | Deposit ingested, then source block hash replaced | No dest approve; row not pending | | A2 | Replacement tx at same height, different hash/nonce | Old row reorged; new tx only after ingest + inclusion | | A3 | `getDeposit` still returns the old hash at latest while canonical block hash diverged | Fail closed on hash check | | A4 | DB row present, `getDeposit` timestamp 0 | Fail closed (today); keep it; also mark reorged | | A5 | Dest approve submitted, dest chain reorgs the approve tx | `reorged`; not treated as confirmed | ## Verification criteria - Operator package tests: T1–T8 and A1–A5. Grep that `block_hash` is compared on the dest-approve / pending-revalidation path (not only INSERT). Grep a production call to `update_approval_reorged` / `update_release_reorged` or the replacement helper (not only the `db/mod.rs` definition). - `get_pending_evm_deposits` cannot return a row whose stored hash failed the last inclusion check. - Do not verify by forcing a BSC/opBNB reorg or by dest-approving a reorged deposit on production. ## Out of scope - [#182](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/182) Terra LCD latest / `TerraConfig.finality_blocks`. - [#115](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/115) RPC quorum, HTTPS, `/health`, `eth_chainId`, raising EVM default N. - [#138](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/138) writer livelock. - [#170](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/170) dest-approved Terra→EVM execute stall. - Solidity `getDeposit` layout / hash words. - Live operator redeploy / RPC URL changes (ops). ## 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.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-bridge-monorepo#183
No description provided.