security(bridge): timelock cancel-window changes and raise 15s floor #177

Open
opened 2026-09-12 11:49:25 +00:00 by PlasticDigits · 0 comments

Summary

Owner/admin can change the watchtower cancel window (and related fee/operator knobs) in one transaction, with a 15 second legal floor. Execute and cancel then read the live global window, not the value that applied at approval. That breaks the documented 5-minute canceler race.

This is not #176 (on-chain M-of-N operator approve). This is not #175 (Terra WithdrawApprove ignoring min_signatures). This is not #135 (Terra wasm / config.admin handoff to the DEX 2-of-3). Closed #114 / #115 are canceler/operator RPC consensus. Closed #44 is frontend rate-limit countdown chrome.

Bundle (same ticket, do not split):

  1. Timelock (at least 24h) on cancel-window / withdraw-delay, fee, and operator-set changes on EVM, Terra, and Solana.
  2. Raise the live floor to a delay watchtowers can actually meet, unless the bridge is paused (15s only if paused, or reject 15s outright).
  3. Snapshot the window at approve time so in-flight rows keep that delay through execute/cancel.
  4. Two-step admin on EVM (Ownable2StepUpgradeable) and Solana (propose/accept, matching Terra propose_admin / accept_admin).

Founder-required contracts / keys / wallet 2-of-3. No community autoland. Do not add ready.

Impact (today vs hypothetical)

Funds at risk today if the admin key is compromised or malicious. The watchtower model in docs/security-model.md assumes a ~5 minute delay so any honest canceler can flag a bad approve. Source already allows the same key that owns the proxy to set the delay to 15 seconds with no notice period. After that, execute/cancel use the new global value, so in-flight approved withdrawals can become executable before a canceler round-trip.

This is not a public user-facing drain by itself: it requires the admin key (EVM owner(), Terra config.admin, Solana bridge.admin). It is still a live privilege-escalation / key-compromise path against the only brake the security model documents for a bad approve. Default in source is 300s (DEFAULT_CANCEL_WINDOW / DEFAULT_WITHDRAW_DELAY); the defect is that shrinking to 15s is an immediate, in-bounds admin call.

Do not publish a mainnet shrink-then-execute sequence.

Current codebase

EVM: setCancelWindow is onlyOwner, 15s–24h, no delay

Bridge.sol is OwnableUpgradeable (not 2-step). Constants:

  • MIN_CANCEL_WINDOW = 15
  • MAX_CANCEL_WINDOW = 24 hours
  • DEFAULT_CANCEL_WINDOW = 5 minutes

setCancelWindow (onlyOwner) writes cancelWindow immediately after the 15s–24h check and emits CancelWindowUpdated. Same pattern for setFeeParams, addOperator / removeOperator, addCanceler / removeCanceler. _authorizeUpgrade is also onlyOwner with an empty body.

PendingWithdraw stores approvedAt but no per-row window. _validateWithdrawExecution and withdrawCancel both use w.approvedAt + cancelWindow (the current storage value).

Forge currently locks in the 15s floor as valid: test_SetCancelWindow_MinBound in Bridge.t.sol calls setCancelWindow(15) and asserts success. There is no test that a mid-flight approve keeps the old window.

Terra: delay setter is instant; admin rotation already has a 7-day timelock

execute_set_withdraw_delay requires info.sender == config.admin, accepts 15..=86400, and WITHDRAW_DELAY.save with no pending proposal.

Cancel and execute load that live item:

let cancel_window = WITHDRAW_DELAY.may_load(...)?.unwrap_or(DEFAULT_WITHDRAW_DELAY);
window_end = pending.approved_at + cancel_window

PendingWithdraw has approved_at only. DEFAULT_WITHDRAW_DELAY = 300.

Admin rotation is already two-step with ADMIN_TIMELOCK_DURATION = 604_800 (propose_admin / accept_admin in execute/admin.rs). That delay does not apply to withdraw-delay, fees, or operator/canceler edits. Do not treat #135 (wasm admin handoff) as this ticket.

Solana: one set_config moves admin, operator, fee, delay, and pause

set_config.rs: signer must be bridge.admin. Optional fields include new_admin, operator, fee_bps, withdraw_delay, paused. Delay must be >= 15 && <= 86400. Admin reassignment is single-step (no pending admin).

PendingWithdraw has approved_at only. withdraw_execute uses pw.approved_at + bridge.withdraw_delay.

Surface Sticky? Scope
Sticky once armed? Until EVM proxy / Terra migrate / Solana program upgrade. 15s is already the tested legal min.
Scope EVM setCancelWindow + execute/cancel; Terra execute_set_withdraw_delay + withdraw execute/cancel; Solana set_config delay/admin/operator/fee; tests + security-model docs. Not operator M-of-N (#176). Not Terra min_signatures (#175).

Invariant that is broken: cancelers have a delay they can actually meet, and that delay cannot be silently shortened under in-flight approvals. Today the admin key can shorten it to 15s and the next execute uses the new value.

Constraints / guardrails

  • Do not remove cancelers, pause, rate limits, or (srcChain, nonce) replay. Execute still requires approved after the window.
  • Pause must remain an immediate owner/admin action (incident brake). Timelock applies to window, fees, and operator/canceler membership, not to pause itself.
  • Raising the floor while unpaused must not stall already-approved rows whose snapshotted window is shorter; those rows keep the approve-time value until execute, cancel, or uncancel (uncancel may start a new snapshotted window — document it).
  • EVM storage: add a per-pending field (or mapping keyed by hash) for the snapshotted window; use __gap / append-only layout so existing pending rows stay loadable. Solana: account resize/migrate must keep live withdraw PDAs loadable. Terra: extend PendingWithdraw without invalidating the active-withdraw index.
  • Terra already has a 7-day admin-rotation timelock. Do not weaken it. Parameter changes need their own ≥24h delay (product may choose 24h vs match the 7-day admin delay; document the choice).
  • EVM two-step ownership must not leave UUPS _authorizeUpgrade as a one-tx owner mint of a malicious implementation; upgrade remains founder-required and should sit behind the same delay or 2-of-3 once that path exists.
  • Frontend / operator / canceler may read getCancelWindow / WITHDRAW_DELAY for UX. After snapshotting, remaining-time for a given hash must use the row window, not only the global config.
  • No community autoland. Do not add ready. No public mainnet shrink-window recipe.

Relevant files

Path Why
packages/contracts-evm/src/Bridge.sol MIN_CANCEL_WINDOW = 15; setCancelWindow onlyOwner; execute/cancel use live cancelWindow
packages/contracts-evm/src/interfaces/IBridge.sol PendingWithdraw has no snapshotted window
packages/contracts-evm/test/Bridge.t.sol test_SetCancelWindow_MinBound asserts 15s is legal
packages/contracts-terraclassic/bridge/src/execute/config.rs Instant execute_set_withdraw_delay (15–86400)
packages/contracts-terraclassic/bridge/src/execute/withdraw.rs Cancel/execute load live WITHDRAW_DELAY
packages/contracts-terraclassic/bridge/src/execute/admin.rs Existing 7-day propose_admin / accept_admin (rotation only)
packages/contracts-terraclassic/bridge/src/state.rs DEFAULT_WITHDRAW_DELAY = 300; pending has approved_at only
packages/contracts-solana/programs/cl8y-bridge/src/instructions/set_config.rs Instant admin/operator/fee/delay/pause
packages/contracts-solana/programs/cl8y-bridge/src/instructions/withdraw_execute.rs approved_at + bridge.withdraw_delay
packages/contracts-solana/programs/cl8y-bridge/src/state/pending_withdraw.rs No snapshotted delay
docs/security-model.md Assumes a 5-minute canceler race; does not mention owner shrinking the window

OpenZeppelin Ownable2Step.sol is already vendored under packages/contracts-evm/lib/openzeppelin-contracts/ (and upgradeable counterparts in the upgradeable package if that is what Bridge imports). Prefer Ownable2StepUpgradeable for the UUPS proxy.

  1. Floor: unpaused minimum delay ≥ what canceler poll + source-chain query + dest inclusion actually needs (product default remains 300s; 15s rejected unless paused). Flip or rewrite test_SetCancelWindow_MinBound and the Terra/Solana 15s-accept tests.
  2. Snapshot: on approve (and on uncancel if the timer resets), store cancelWindow / withdraw_delay on the pending row. Execute and cancel use that field only.
  3. Timelock: pending-config pattern (propose → execute after ≥24h) for window, fee params, and operator/canceler add/remove on all three chains. Pause stays immediate.
  4. Two-step admin: EVM Ownable2StepUpgradeable; Solana pending-admin + accept (mirror Terra). Instant new_admin in set_config goes away.
  5. Update docs/security-model.md so “time to respond” is a protocol floor, not a deploy-time hope, and so owner key compromise includes “cannot silently collapse the window.”

Acceptance criteria

  • AC1. Unpaused, setCancelWindow(15) / Terra delay_seconds = 15 / Solana withdraw_delay = 15 reverts (or is allowed only while paused and cannot be used to execute).
  • AC2. Window/fee/operator (and canceler membership) changes cannot take effect in the same transaction as the propose; they become live only after ≥24h (or the documented longer delay). Pause remains immediate.
  • AC3. Approve at window W, then admin (after timelock) sets window W′ < remaining time: that row still cannot execute until approvedAt + W, and cancel remains valid until approvedAt + W.
  • AC4. EVM transferOwnership / Solana new_admin does not complete until the new admin accepts. Terra 7-day admin timelock still holds.
  • AC5. Existing happy-path withdraw tests (approve, cancel inside window, execute after default 300s) stay green at the default delay.
  • AC6. Docs no longer describe a guaranteed 5-minute race while source still permits a 15s live floor.

Verification (non-exploitative)

Local Forge / CosmWasm / Anchor tests only. Do not publish a mainnet owner call that shortens the window.

  1. EVM: setCancelWindow(15) reverts while unpaused; 14 still reverts; paused-only 15s if that exception is chosen.
  2. EVM: approve at 300s, warp 20s, set window to the new floor (after timelock in the same test via vm.warp on the timelock, not by skipping it). Execute still reverts until approvedAt + 300. Cancel still succeeds inside 300s.
  3. Terra: same matrix on execute_set_withdraw_delay + WithdrawCancel / execute-unlock.
  4. Solana: set_config delay 15 reverts while unpaused; in-flight PDA keeps snapshotted delay; new_admin does not stick until accept.
  5. Timelock: propose then immediate execute reverts; after warp ≥24h the new window applies to new approvals only.
  6. Regression: default 300s approve → cancel / execute paths unchanged.

First-pass model recommendation

Recommendation: grok-high

Rationale: Security class plus founder-required contracts, admin keys, and wallet / 2-of-3. Composer is disallowed (High/security; contracts/auth/keys; three chains plus pending-account layout; timelock + snapshot is a protocol/state change, not a local three-file edit). A wrong snapshot or timelock skip either stalls honest withdraws or restores the 15s race. Verify with Forge + CosmWasm + Anchor tests, not a live window change.

## Summary Owner/admin can change the watchtower cancel window (and related fee/operator knobs) in one transaction, with a **15 second** legal floor. Execute and cancel then read the **live** global window, not the value that applied at approval. That breaks the documented 5-minute canceler race. This is **not** [#176](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/176) (on-chain M-of-N operator approve). This is **not** [#175](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/175) (Terra `WithdrawApprove` ignoring `min_signatures`). This is **not** [#135](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/135) (Terra wasm / `config.admin` handoff to the DEX 2-of-3). Closed [#114](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/114) / [#115](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/115) are canceler/operator RPC consensus. Closed [#44](https://git.cl8y.com/code/cl8y-bridge-monorepo/issues/44) is frontend rate-limit countdown chrome. Bundle (same ticket, do not split): 1. **Timelock** (at least 24h) on cancel-window / withdraw-delay, fee, and operator-set changes on EVM, Terra, and Solana. 2. **Raise the live floor** to a delay watchtowers can actually meet, unless the bridge is paused (15s only if paused, or reject 15s outright). 3. **Snapshot** the window at approve time so in-flight rows keep that delay through execute/cancel. 4. **Two-step admin** on EVM (`Ownable2StepUpgradeable`) and Solana (propose/accept, matching Terra `propose_admin` / `accept_admin`). Founder-required contracts / keys / wallet 2-of-3. No community autoland. Do not add `ready`. ## Impact (today vs hypothetical) **Funds at risk today if the admin key is compromised or malicious.** The watchtower model in [`docs/security-model.md`](docs/security-model.md) assumes a ~5 minute delay so any honest canceler can flag a bad approve. Source already allows the same key that owns the proxy to set the delay to **15 seconds** with no notice period. After that, execute/cancel use the new global value, so in-flight approved withdrawals can become executable before a canceler round-trip. This is not a public user-facing drain by itself: it requires the admin key (EVM `owner()`, Terra `config.admin`, Solana `bridge.admin`). It is still a live privilege-escalation / key-compromise path against the only brake the security model documents for a bad approve. Default in source is 300s (`DEFAULT_CANCEL_WINDOW` / `DEFAULT_WITHDRAW_DELAY`); the defect is that shrinking to 15s is an immediate, in-bounds admin call. Do not publish a mainnet shrink-then-execute sequence. ## Current codebase ### EVM: `setCancelWindow` is `onlyOwner`, 15s–24h, no delay [`Bridge.sol`](packages/contracts-evm/src/Bridge.sol) is `OwnableUpgradeable` (not 2-step). Constants: - `MIN_CANCEL_WINDOW = 15` - `MAX_CANCEL_WINDOW = 24 hours` - `DEFAULT_CANCEL_WINDOW = 5 minutes` `setCancelWindow` (`onlyOwner`) writes `cancelWindow` immediately after the 15s–24h check and emits `CancelWindowUpdated`. Same pattern for `setFeeParams`, `addOperator` / `removeOperator`, `addCanceler` / `removeCanceler`. `_authorizeUpgrade` is also `onlyOwner` with an empty body. [`PendingWithdraw`](packages/contracts-evm/src/interfaces/IBridge.sol) stores `approvedAt` but **no per-row window**. `_validateWithdrawExecution` and `withdrawCancel` both use `w.approvedAt + cancelWindow` (the **current** storage value). Forge currently **locks in** the 15s floor as valid: `test_SetCancelWindow_MinBound` in [`Bridge.t.sol`](packages/contracts-evm/test/Bridge.t.sol) calls `setCancelWindow(15)` and asserts success. There is no test that a mid-flight approve keeps the old window. ### Terra: delay setter is instant; admin rotation already has a 7-day timelock [`execute_set_withdraw_delay`](packages/contracts-terraclassic/bridge/src/execute/config.rs) requires `info.sender == config.admin`, accepts `15..=86400`, and `WITHDRAW_DELAY.save` with no pending proposal. Cancel and execute load that live item: ```text let cancel_window = WITHDRAW_DELAY.may_load(...)?.unwrap_or(DEFAULT_WITHDRAW_DELAY); window_end = pending.approved_at + cancel_window ``` [`PendingWithdraw`](packages/contracts-terraclassic/bridge/src/state.rs) has `approved_at` only. `DEFAULT_WITHDRAW_DELAY = 300`. Admin **rotation** is already two-step with `ADMIN_TIMELOCK_DURATION = 604_800` (`propose_admin` / `accept_admin` in [`execute/admin.rs`](packages/contracts-terraclassic/bridge/src/execute/admin.rs)). That delay does **not** apply to withdraw-delay, fees, or operator/canceler edits. Do not treat #135 (wasm admin handoff) as this ticket. ### Solana: one `set_config` moves admin, operator, fee, delay, and pause [`set_config.rs`](packages/contracts-solana/programs/cl8y-bridge/src/instructions/set_config.rs): signer must be `bridge.admin`. Optional fields include `new_admin`, `operator`, `fee_bps`, `withdraw_delay`, `paused`. Delay must be `>= 15 && <= 86400`. Admin reassignment is **single-step** (no pending admin). [`PendingWithdraw`](packages/contracts-solana/programs/cl8y-bridge/src/state/pending_withdraw.rs) has `approved_at` only. [`withdraw_execute`](packages/contracts-solana/programs/cl8y-bridge/src/instructions/withdraw_execute.rs) uses `pw.approved_at + bridge.withdraw_delay`. | Surface | Sticky? | Scope | | --- | --- | --- | | Sticky once armed? | Until EVM proxy / Terra migrate / Solana program upgrade. 15s is already the tested legal min. | | Scope | EVM `setCancelWindow` + execute/cancel; Terra `execute_set_withdraw_delay` + withdraw execute/cancel; Solana `set_config` delay/admin/operator/fee; tests + security-model docs. Not operator M-of-N (#176). Not Terra `min_signatures` (#175). | Invariant that is broken: cancelers have a delay they can actually meet, and that delay cannot be silently shortened under in-flight approvals. Today the admin key can shorten it to 15s and the next execute uses the new value. ## Constraints / guardrails - Do not remove cancelers, pause, rate limits, or `(srcChain, nonce)` replay. Execute still requires `approved` after the window. - Pause must remain an **immediate** owner/admin action (incident brake). Timelock applies to window, fees, and operator/canceler membership, not to pause itself. - Raising the floor while unpaused must not stall already-approved rows whose snapshotted window is shorter; those rows keep the approve-time value until execute, cancel, or uncancel (uncancel may start a **new** snapshotted window — document it). - EVM storage: add a per-pending field (or mapping keyed by hash) for the snapshotted window; use `__gap` / append-only layout so existing pending rows stay loadable. Solana: account resize/migrate must keep live withdraw PDAs loadable. Terra: extend `PendingWithdraw` without invalidating the active-withdraw index. - Terra already has a 7-day admin-rotation timelock. Do not weaken it. Parameter changes need their own ≥24h delay (product may choose 24h vs match the 7-day admin delay; document the choice). - EVM two-step ownership must not leave UUPS `_authorizeUpgrade` as a one-tx owner mint of a malicious implementation; upgrade remains founder-required and should sit behind the same delay or 2-of-3 once that path exists. - Frontend / operator / canceler may read `getCancelWindow` / `WITHDRAW_DELAY` for UX. After snapshotting, remaining-time for a given hash must use the **row** window, not only the global config. - No community autoland. Do not add `ready`. No public mainnet shrink-window recipe. ## Relevant files | Path | Why | | --- | --- | | `packages/contracts-evm/src/Bridge.sol` | `MIN_CANCEL_WINDOW = 15`; `setCancelWindow` `onlyOwner`; execute/cancel use live `cancelWindow` | | `packages/contracts-evm/src/interfaces/IBridge.sol` | `PendingWithdraw` has no snapshotted window | | `packages/contracts-evm/test/Bridge.t.sol` | `test_SetCancelWindow_MinBound` asserts 15s is legal | | `packages/contracts-terraclassic/bridge/src/execute/config.rs` | Instant `execute_set_withdraw_delay` (15–86400) | | `packages/contracts-terraclassic/bridge/src/execute/withdraw.rs` | Cancel/execute load live `WITHDRAW_DELAY` | | `packages/contracts-terraclassic/bridge/src/execute/admin.rs` | Existing 7-day `propose_admin` / `accept_admin` (rotation only) | | `packages/contracts-terraclassic/bridge/src/state.rs` | `DEFAULT_WITHDRAW_DELAY = 300`; pending has `approved_at` only | | `packages/contracts-solana/programs/cl8y-bridge/src/instructions/set_config.rs` | Instant admin/operator/fee/delay/pause | | `packages/contracts-solana/programs/cl8y-bridge/src/instructions/withdraw_execute.rs` | `approved_at + bridge.withdraw_delay` | | `packages/contracts-solana/programs/cl8y-bridge/src/state/pending_withdraw.rs` | No snapshotted delay | | `docs/security-model.md` | Assumes a 5-minute canceler race; does not mention owner shrinking the window | OpenZeppelin `Ownable2Step.sol` is already vendored under `packages/contracts-evm/lib/openzeppelin-contracts/` (and upgradeable counterparts in the upgradeable package if that is what `Bridge` imports). Prefer `Ownable2StepUpgradeable` for the UUPS proxy. ## Recommended direction 1. **Floor:** unpaused minimum delay ≥ what canceler poll + source-chain query + dest inclusion actually needs (product default remains 300s; 15s rejected unless `paused`). Flip or rewrite `test_SetCancelWindow_MinBound` and the Terra/Solana 15s-accept tests. 2. **Snapshot:** on approve (and on uncancel if the timer resets), store `cancelWindow` / `withdraw_delay` on the pending row. Execute and cancel use that field only. 3. **Timelock:** pending-config pattern (propose → execute after ≥24h) for window, fee params, and operator/canceler add/remove on all three chains. Pause stays immediate. 4. **Two-step admin:** EVM `Ownable2StepUpgradeable`; Solana pending-admin + accept (mirror Terra). Instant `new_admin` in `set_config` goes away. 5. Update `docs/security-model.md` so “time to respond” is a protocol floor, not a deploy-time hope, and so owner key compromise includes “cannot silently collapse the window.” ## Acceptance criteria - AC1. Unpaused, `setCancelWindow(15)` / Terra `delay_seconds = 15` / Solana `withdraw_delay = 15` reverts (or is allowed only while paused and cannot be used to execute). - AC2. Window/fee/operator (and canceler membership) changes cannot take effect in the same transaction as the propose; they become live only after ≥24h (or the documented longer delay). Pause remains immediate. - AC3. Approve at window W, then admin (after timelock) sets window W′ < remaining time: that row still cannot execute until `approvedAt + W`, and cancel remains valid until `approvedAt + W`. - AC4. EVM `transferOwnership` / Solana `new_admin` does not complete until the new admin accepts. Terra 7-day admin timelock still holds. - AC5. Existing happy-path withdraw tests (approve, cancel inside window, execute after default 300s) stay green at the default delay. - AC6. Docs no longer describe a guaranteed 5-minute race while source still permits a 15s live floor. ## Verification (non-exploitative) Local Forge / CosmWasm / Anchor tests only. Do not publish a mainnet owner call that shortens the window. 1. EVM: `setCancelWindow(15)` reverts while unpaused; `14` still reverts; paused-only 15s if that exception is chosen. 2. EVM: approve at 300s, warp 20s, set window to the new floor (after timelock in the same test via `vm.warp` on the **timelock**, not by skipping it). Execute still reverts until `approvedAt + 300`. Cancel still succeeds inside 300s. 3. Terra: same matrix on `execute_set_withdraw_delay` + `WithdrawCancel` / execute-unlock. 4. Solana: `set_config` delay 15 reverts while unpaused; in-flight PDA keeps snapshotted delay; `new_admin` does not stick until accept. 5. Timelock: propose then immediate execute reverts; after warp ≥24h the new window applies to **new** approvals only. 6. Regression: default 300s approve → cancel / execute paths unchanged. ## First-pass model recommendation Recommendation: grok-high Rationale: Security class plus founder-required contracts, admin keys, and wallet / 2-of-3. Composer is disallowed (High/security; contracts/auth/keys; three chains plus pending-account layout; timelock + snapshot is a protocol/state change, not a local three-file edit). A wrong snapshot or timelock skip either stalls honest withdraws or restores the 15s race. Verify with Forge + CosmWasm + Anchor tests, not a live window change.
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#177
No description provided.