code/cl8y-bridge-monorepo
4
0
Fork 0
Code Issues 38 Pull requests 4 Projects Releases Packages Wiki Activity

security(bridge): pause must stop withdraw approve or reset cancel window #188

New issue
Open
opened 2026-09-12 13:03:16 +00:00 by PlasticDigits · 0 comments
PlasticDigits commented 2026-09-12 13:03:16 +00:00
Owner
Copy link

Summary

Incident pause stops execute (and user deposit/submit) but does not stop operator approve on EVM or Terra. approvedAt is wall-clock. A pause that outlasts the remaining cancel window lets execute succeed immediately on unpause, while cancelers may have treated pause as “nothing is moving.”

Solana already rejects withdraw_approve while bridge.paused. EVM withdrawApprove has no whenNotPaused. Terra execute_withdraw_approve loads CONFIG and never reads config.paused. Unpause on all three chains only flips the flag; it does not freeze or restart in-flight windows.

This is not #177 (admin setCancelWindow / 15s floor / snapshot at approve). This is not #176 (M-of-N operator approve) or #175 (Terra min_signatures). This is not #179 (recoverAsset native sweep while paused). Keyword overlap on “pause” / approvedAt is not this bug.

Internal review id: S-3. Still in source as of 2026-09-12.

Bundle (same ticket, do not split):

  1. Reject new withdraw approvals while paused on EVM and Terra (Solana already does).
  2. Freeze or restart the cancel window across unpause for every approved-but-unexecuted row (all three chains), so pause duration cannot consume the watchtower race.
  3. Tests: approve-while-paused reverts; pause longer than the remaining window, unpause, execute still blocked until a full (or remaining frozen) window elapses.
  4. Docs: pause is an incident brake on approve and the cancel clock, not only execute.

Founder-required contracts. No community autoland. Do not add ready.

Impact (today vs hypothetical)

Question Answer
Funds at risk today? No permissionless user drain. Compromised or confused operator can approve during an admin pause. After pause ≥ remaining cancelWindow / withdraw_delay, anyone can execute on unpause with no further canceler opportunity.
Auth / admin required? Approve still needs EVM onlyOperator / Terra operator-or-admin / Solana bridge.operator. Pause/unpause is owner/admin. The hole is that pause does not stop the operator path or the clock.
Theft vs lock? Incident pause is supposed to halt outbound mint/unlock while humans investigate. Today it only delays execute. Approvals taken during pause (EVM/Terra) or before pause (all chains) can become immediately executable when the flag clears. Not a lock bug.
Sticky once armed? Until EVM proxy / Terra wasm / Solana program upgrade. Live pause + unpause already exhibits the clock behavior.
Scope EVM withdrawApprove + pause/unpause; Terra execute_withdraw_approve + execute_unpause; Solana unpause clock (approve already gated); cancel must remain possible during pause; tests + security-model / Terra pause docs. Not M-of-N (#176/#175). Not recoverAsset (#179).

Invariant that is broken: admin pause is a full incident brake on the withdraw pipeline. Cancelers get a delay they can meet after the incident, not a window that ticks while they assume the bridge is frozen.

Docs already over-claim this: Terra execute_pause comment and docs/contracts-terraclassic.md say pause “stops all transfers” / “all bridge operations.” EVM tests only prove deposit reverts while paused. Solana INV-D2 says paused withdraw paths reject; approve is covered there, the unpause clock is not.

Do not publish a mainnet pause-then-approve-then-unpause-execute sequence.

EVM: execute has whenNotPaused; approve does not

pause / unpause are onlyOwner wrappers around OpenZeppelin _pause / _unpause. They do not touch pendingWithdraws.

Function Pause gate
depositNative / depositERC20 / withdrawSubmit whenNotPaused
withdrawExecuteUnlock / withdrawExecuteMint whenNotPaused
withdrawApprove onlyOperator nonReentrant only
withdrawCancel / withdrawUncancel no pause modifier

withdrawApprove sets approved = true, approvedAt = block.timestamp, marks withdrawNonceUsed. _validateWithdrawExecution uses w.approvedAt + cancelWindow against current block.timestamp. Pause does not stop that clock.

test_Pause / test_Unpause in Bridge.t.sol only cover depositERC20. There is no withdrawApprove while paused. withdrawUncancel already resets approvedAt (comment: “restart cancel window”) — that pattern is the right shape for unpause, but unpause does not use it.

Terra: submit/execute check config.paused; approve does not

execute_withdraw_submit, execute_withdraw_execute_unlock, and execute_withdraw_execute_mint return ContractError::BridgePaused.

execute_withdraw_approve loads CONFIG for the operator/admin check, then:

  • pending.approved = true
  • pending.approved_at = env.block.time.seconds()
  • save_pending_and_sync_index + WITHDRAW_NONCE_USED

No config.paused read. execute_withdraw_cancel also has no pause check (cancelers must remain able to cancel during an incident). execute_unpause only sets config.paused = false.

Tests: test_execute_while_paused_rejected and test_submit_while_paused_rejected exist. There is no approve-while-paused test. test_execute_while_paused_rejected even approves first, waits, then pauses — it never asks whether approve itself is legal during pause.

Solana: approve already paused; unpause still burns the window

withdraw_approve::handler has require!(!bridge.paused, BridgeError::BridgePaused). Tests include withdraw_approve rejects when paused. Execute also checks pause. withdraw_cancel does not (keep that).

set_config writes bridge.paused with no approved_at adjustment. Rows approved before pause still age during pause; unpause then allows execute if Clock + delay already passed. INV-D2 does not mention the clock.

Constraints / guardrails

  • Do not remove cancelers, rate limits, (srcChain, nonce) replay, or execute-after-window. Execute still requires approved and whenNotPaused.
  • Cancel must keep working while paused. A pause that also blocks withdrawCancel traps cancelers for the entire incident. EVM/Terra/Solana cancel stay ungated (or explicitly gated only if product documents a substitute).
  • Block new approve while paused on EVM and Terra. Prefer also blocking withdrawUncancel while paused (it starts a new approvedAt). Do not treat uncancel as an incident bypass.
  • Unpause must not make in-flight approved rows immediately executable. Two allowed designs (pick one, test it, document it):
    1. Restart: on unpause, set approvedAt / approved_at = now for every approved, not executed, not cancelled row (full new window). Matches the asked behavior; delays honest rows by a full window after every pause.
    2. Freeze: accumulate paused duration (or max(approvedAt, lastUnpausedAt)) so remaining time is preserved. O(1) via a bridge-level pausedAt / pauseElapsed is preferred on Solana (pending PDAs are not cheap to enumerate).
  • EVM: do not break _pendingWithdrawHashes iteration / __gap. Terra: do not break INV-TC-AW1 (ACTIVE_WITHDRAW_HASHES). Solana: do not require a full PDA walk if a config-level clock works.
  • Pause itself stays an immediate owner/admin action (same as #177). Do not timelock pause.
  • Operator writers / frontend may keep sending approve during pause; they must revert cleanly (Pausable: paused / BridgePaused). Out of scope to redesign the writer, but do not claim pause is a complete brake until the chain rejects approve.
  • No community autoland. Do not add ready. No public mainnet incident recipe.

Relevant files

Path Why
packages/contracts-evm/src/Bridge.sol withdrawApprove lacks whenNotPaused; pause/unpause do not touch approvedAt
packages/contracts-evm/test/Bridge.t.sol test_Pause is deposit-only
packages/contracts-terraclassic/bridge/src/execute/withdraw.rs execute_withdraw_approve ignores config.paused
packages/contracts-terraclassic/bridge/src/execute/admin.rs execute_unpause flag-only; comment claims pause stops all transfers
packages/contracts-terraclassic/bridge/tests/test_withdraw_flow.rs Submit/execute paused tests; no approve-paused
packages/contracts-solana/programs/cl8y-bridge/src/instructions/withdraw_approve.rs Already paused — keep
packages/contracts-solana/programs/cl8y-bridge/src/instructions/set_config.rs Unpause does not reset/freeze windows
packages/contracts-solana/tests/deposit_withdraw.test.ts Approve-paused already covered
docs/security-model.md Admin “pause if needed” does not mention the cancel clock
docs/contracts-terraclassic.md “Pause all bridge operations” is false for approve
docs/SOLANA_BRIDGE_INVARIANTS.md INV-D2: extend with unpause clock

Recommended direction

  1. EVM: add whenNotPaused to withdrawApprove (and withdrawUncancel). Terra: if config.paused { BridgePaused } at the top of execute_withdraw_approve (and uncancel). Leave cancel ungated.
  2. Unpause clock: implement freeze or restart on all three chains in the same PR. Prefer a bridge-level pause accumulator so Solana does not scan PDAs. If restart is chosen, iterate EVM _pendingWithdrawHashes and Terra ACTIVE_WITHDRAW_HASHES; Solana still needs a config-level effective start (max(approved_at, last_unpaused_at)).
  3. Flip/add tests listed below. Extend test_Pause so it is not deposit-only.
  4. Correct Terra/EVM/Solana docs so pause is specified as: no deposit/submit/approve/execute; cancel allowed; window does not elapse for execute.

Acceptance criteria

  • AC1. EVM withdrawApprove while paused reverts (Pausable / whenNotPaused). Terra WithdrawApprove while paused returns BridgePaused. Solana approve-paused tests stay green.
  • AC2. EVM/Terra/Solana: approve (unpaused), warp just inside the window, pause, warp past the original approvedAt + delay, unpause: execute still reverts until the frozen remainder or a restarted full window has elapsed.
  • AC3. Cancel while paused still succeeds for an in-window approved row (do not lock cancelers).
  • AC4. Uncancel while paused reverts (or is documented + tested as resetting only after unpause — default is revert).
  • AC5. Existing happy-path approve → wait 300s → execute stays green when never paused.
  • AC6. Docs no longer say pause stops “all operations” while approve is ungated, and they describe the unpause clock.

Verification (non-exploitative)

Local Forge / CosmWasm / Anchor tests only. Do not pause a production proxy or publish an incident sequence.

  1. EVM: pause → withdrawApprove reverts; unpause → approve succeeds. Deposit/execute paused tests stay green.
  2. EVM: approve at t0, warp +10s, pause, warp + default window, unpause, execute reverts; warp the remainder or full restarted window, then execute succeeds.
  3. Terra: same matrix on Pause / WithdrawApprove / WithdrawExecuteUnlock. Add test_approve_while_paused_rejected next to the existing submit/execute paused tests.
  4. Solana: keep approve-paused; add unpause-clock test (approve, pause, advance beyond delay, unpause, execute still CancelWindow/too early).
  5. Cancel-while-paused: one positive test per chain.
  6. Regression: default 300s unpaused path unchanged.

First-pass model recommendation

Recommendation: grok-high

Rationale: Security class plus founder-required contracts (EVM proxy, Terra wasm, Solana program), pause/auth, and cancel-window protocol state. Composer is disallowed (High/security; contracts/keys; pause clock is a cross-chain state/protocol change, not a local three-file edit). A missed unpause reset or a paused cancel gate either restores immediate execute or traps honest cancelers. Verify with Forge + CosmWasm + Anchor tests, not a live pause.

## Summary Incident pause stops **execute** (and user deposit/submit) but does **not** stop **operator approve** on EVM or Terra. `approvedAt` is wall-clock. A pause that outlasts the remaining cancel window lets execute succeed immediately on unpause, while cancelers may have treated pause as “nothing is moving.” Solana already rejects `withdraw_approve` while `bridge.paused`. EVM `withdrawApprove` has no `whenNotPaused`. Terra `execute_withdraw_approve` loads `CONFIG` and never reads `config.paused`. Unpause on all three chains only flips the flag; it does not freeze or restart in-flight windows. This is not [#177](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/177) (admin `setCancelWindow` / 15s floor / snapshot at approve). This is not [#176](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/176) (M-of-N operator approve) or [#175](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/175) (Terra `min_signatures`). This is not [#179](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/179) (`recoverAsset` native sweep while paused). Keyword overlap on “pause” / `approvedAt` is not this bug. Internal review id: S-3. Still in source as of 2026-09-12. Bundle (same ticket, do not split): 1. Reject new withdraw approvals while paused on EVM and Terra (Solana already does). 2. Freeze or restart the cancel window across **unpause** for every approved-but-unexecuted row (all three chains), so pause duration cannot consume the watchtower race. 3. Tests: approve-while-paused reverts; pause longer than the remaining window, unpause, execute still blocked until a full (or remaining frozen) window elapses. 4. Docs: pause is an incident brake on approve **and** the cancel clock, not only execute. Founder-required contracts. No community autoland. Do not add `ready`. ## Impact (today vs hypothetical) | Question | Answer | | --- | --- | | Funds at risk today? | No permissionless user drain. Compromised or confused **operator** can approve during an admin pause. After pause ≥ remaining `cancelWindow` / `withdraw_delay`, **anyone** can execute on unpause with no further canceler opportunity. | | Auth / admin required? | Approve still needs EVM `onlyOperator` / Terra operator-or-admin / Solana `bridge.operator`. Pause/unpause is owner/admin. The hole is that pause does not stop the operator path or the clock. | | Theft vs lock? | Incident pause is supposed to halt outbound mint/unlock while humans investigate. Today it only delays execute. Approvals taken during pause (EVM/Terra) or before pause (all chains) can become immediately executable when the flag clears. Not a lock bug. | | Sticky once armed? | Until EVM proxy / Terra wasm / Solana program upgrade. Live pause + unpause already exhibits the clock behavior. | | Scope | EVM `withdrawApprove` + `pause`/`unpause`; Terra `execute_withdraw_approve` + `execute_unpause`; Solana unpause clock (approve already gated); cancel must remain possible during pause; tests + security-model / Terra pause docs. Not M-of-N (#176/#175). Not `recoverAsset` (#179). | Invariant that is broken: admin pause is a full incident brake on the withdraw pipeline. Cancelers get a delay they can meet **after** the incident, not a window that ticks while they assume the bridge is frozen. Docs already over-claim this: Terra `execute_pause` comment and `docs/contracts-terraclassic.md` say pause “stops all transfers” / “all bridge operations.” EVM tests only prove deposit reverts while paused. Solana INV-D2 says paused withdraw paths reject; approve is covered there, the unpause clock is not. Do not publish a mainnet pause-then-approve-then-unpause-execute sequence. ### EVM: execute has `whenNotPaused`; approve does not `pause` / `unpause` are `onlyOwner` wrappers around OpenZeppelin `_pause` / `_unpause`. They do not touch `pendingWithdraws`. | Function | Pause gate | | --- | --- | | `depositNative` / `depositERC20` / `withdrawSubmit` | `whenNotPaused` | | `withdrawExecuteUnlock` / `withdrawExecuteMint` | `whenNotPaused` | | `withdrawApprove` | `onlyOperator nonReentrant` only | | `withdrawCancel` / `withdrawUncancel` | no pause modifier | `withdrawApprove` sets `approved = true`, `approvedAt = block.timestamp`, marks `withdrawNonceUsed`. `_validateWithdrawExecution` uses `w.approvedAt + cancelWindow` against **current** `block.timestamp`. Pause does not stop that clock. `test_Pause` / `test_Unpause` in `Bridge.t.sol` only cover `depositERC20`. There is no `withdrawApprove` while paused. `withdrawUncancel` already resets `approvedAt` (comment: “restart cancel window”) — that pattern is the right shape for unpause, but unpause does not use it. ### Terra: submit/execute check `config.paused`; approve does not `execute_withdraw_submit`, `execute_withdraw_execute_unlock`, and `execute_withdraw_execute_mint` return `ContractError::BridgePaused`. `execute_withdraw_approve` loads `CONFIG` for the operator/admin check, then: - `pending.approved = true` - `pending.approved_at = env.block.time.seconds()` - `save_pending_and_sync_index` + `WITHDRAW_NONCE_USED` No `config.paused` read. `execute_withdraw_cancel` also has no pause check (cancelers must remain able to cancel during an incident). `execute_unpause` only sets `config.paused = false`. Tests: `test_execute_while_paused_rejected` and `test_submit_while_paused_rejected` exist. There is no approve-while-paused test. `test_execute_while_paused_rejected` even **approves first**, waits, **then** pauses — it never asks whether approve itself is legal during pause. ### Solana: approve already paused; unpause still burns the window `withdraw_approve::handler` has `require!(!bridge.paused, BridgeError::BridgePaused)`. Tests include `withdraw_approve rejects when paused`. Execute also checks pause. `withdraw_cancel` does not (keep that). `set_config` writes `bridge.paused` with no `approved_at` adjustment. Rows approved **before** pause still age during pause; unpause then allows execute if `Clock + delay` already passed. INV-D2 does not mention the clock. ## Constraints / guardrails - Do not remove cancelers, rate limits, `(srcChain, nonce)` replay, or execute-after-window. Execute still requires `approved` and `whenNotPaused`. - **Cancel must keep working while paused.** A pause that also blocks `withdrawCancel` traps cancelers for the entire incident. EVM/Terra/Solana cancel stay ungated (or explicitly gated only if product documents a substitute). - Block **new** approve while paused on EVM and Terra. Prefer also blocking `withdrawUncancel` while paused (it starts a new `approvedAt`). Do not treat uncancel as an incident bypass. - Unpause must not make in-flight approved rows immediately executable. Two allowed designs (pick one, test it, document it): 1. **Restart:** on unpause, set `approvedAt` / `approved_at` = now for every approved, not executed, not cancelled row (full new window). Matches the asked behavior; delays honest rows by a full window after every pause. 2. **Freeze:** accumulate paused duration (or `max(approvedAt, lastUnpausedAt)`) so remaining time is preserved. O(1) via a bridge-level `pausedAt` / `pauseElapsed` is preferred on Solana (pending PDAs are not cheap to enumerate). - EVM: do not break `_pendingWithdrawHashes` iteration / `__gap`. Terra: do not break INV-TC-AW1 (`ACTIVE_WITHDRAW_HASHES`). Solana: do not require a full PDA walk if a config-level clock works. - Pause itself stays an **immediate** owner/admin action (same as #177). Do not timelock pause. - Operator writers / frontend may keep sending approve during pause; they must revert cleanly (`Pausable: paused` / `BridgePaused`). Out of scope to redesign the writer, but do not claim pause is a complete brake until the chain rejects approve. - No community autoland. Do not add `ready`. No public mainnet incident recipe. ## Relevant files | Path | Why | | --- | --- | | `packages/contracts-evm/src/Bridge.sol` | `withdrawApprove` lacks `whenNotPaused`; `pause`/`unpause` do not touch `approvedAt` | | `packages/contracts-evm/test/Bridge.t.sol` | `test_Pause` is deposit-only | | `packages/contracts-terraclassic/bridge/src/execute/withdraw.rs` | `execute_withdraw_approve` ignores `config.paused` | | `packages/contracts-terraclassic/bridge/src/execute/admin.rs` | `execute_unpause` flag-only; comment claims pause stops all transfers | | `packages/contracts-terraclassic/bridge/tests/test_withdraw_flow.rs` | Submit/execute paused tests; no approve-paused | | `packages/contracts-solana/programs/cl8y-bridge/src/instructions/withdraw_approve.rs` | Already paused — keep | | `packages/contracts-solana/programs/cl8y-bridge/src/instructions/set_config.rs` | Unpause does not reset/freeze windows | | `packages/contracts-solana/tests/deposit_withdraw.test.ts` | Approve-paused already covered | | `docs/security-model.md` | Admin “pause if needed” does not mention the cancel clock | | `docs/contracts-terraclassic.md` | “Pause all bridge operations” is false for approve | | `docs/SOLANA_BRIDGE_INVARIANTS.md` | INV-D2: extend with unpause clock | ## Recommended direction 1. EVM: add `whenNotPaused` to `withdrawApprove` (and `withdrawUncancel`). Terra: `if config.paused { BridgePaused }` at the top of `execute_withdraw_approve` (and uncancel). Leave cancel ungated. 2. Unpause clock: implement freeze **or** restart on all three chains in the same PR. Prefer a bridge-level pause accumulator so Solana does not scan PDAs. If restart is chosen, iterate EVM `_pendingWithdrawHashes` and Terra `ACTIVE_WITHDRAW_HASHES`; Solana still needs a config-level effective start (`max(approved_at, last_unpaused_at)`). 3. Flip/add tests listed below. Extend `test_Pause` so it is not deposit-only. 4. Correct Terra/EVM/Solana docs so pause is specified as: no deposit/submit/approve/execute; cancel allowed; window does not elapse for execute. ## Acceptance criteria - AC1. EVM `withdrawApprove` while paused reverts (`Pausable` / `whenNotPaused`). Terra `WithdrawApprove` while paused returns `BridgePaused`. Solana approve-paused tests stay green. - AC2. EVM/Terra/Solana: approve (unpaused), warp just inside the window, pause, warp **past** the original `approvedAt + delay`, unpause: execute still reverts until the frozen remainder or a restarted full window has elapsed. - AC3. Cancel while paused still succeeds for an in-window approved row (do not lock cancelers). - AC4. Uncancel while paused reverts (or is documented + tested as resetting only after unpause — default is revert). - AC5. Existing happy-path approve → wait 300s → execute stays green when never paused. - AC6. Docs no longer say pause stops “all operations” while approve is ungated, and they describe the unpause clock. ## Verification (non-exploitative) Local Forge / CosmWasm / Anchor tests only. Do not pause a production proxy or publish an incident sequence. 1. EVM: pause → `withdrawApprove` reverts; unpause → approve succeeds. Deposit/execute paused tests stay green. 2. EVM: approve at t0, warp +10s, pause, warp + default window, unpause, execute reverts; warp the remainder or full restarted window, then execute succeeds. 3. Terra: same matrix on `Pause` / `WithdrawApprove` / `WithdrawExecuteUnlock`. Add `test_approve_while_paused_rejected` next to the existing submit/execute paused tests. 4. Solana: keep approve-paused; add unpause-clock test (approve, pause, advance beyond delay, unpause, execute still `CancelWindow`/`too early`). 5. Cancel-while-paused: one positive test per chain. 6. Regression: default 300s unpaused path unchanged. ## First-pass model recommendation Recommendation: grok-high Rationale: Security class plus founder-required contracts (EVM proxy, Terra wasm, Solana program), pause/auth, and cancel-window protocol state. Composer is disallowed (High/security; contracts/keys; pause clock is a cross-chain state/protocol change, not a local three-file edit). A missed unpause reset or a paused cancel gate either restores immediate execute or traps honest cancelers. Verify with Forge + CosmWasm + Anchor tests, not a live pause.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
issue/165
issue/187
issue/170
issue/195
renovate/configure
review/brouie-solana-e2e-70
Labels
Clear labels   
agent:implement
Queue implement gate

  
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
Definition of 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 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
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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#188
No description provided.