security(bridge): timelock cancel-window changes and raise 15s floor #177
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#177
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
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
WithdrawApproveignoringmin_signatures). This is not #135 (Terra wasm /config.adminhandoff 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):
Ownable2StepUpgradeable) and Solana (propose/accept, matching Terrapropose_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.mdassumes 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(), Terraconfig.admin, Solanabridge.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:
setCancelWindowisonlyOwner, 15s–24h, no delayBridge.solisOwnableUpgradeable(not 2-step). Constants:MIN_CANCEL_WINDOW = 15MAX_CANCEL_WINDOW = 24 hoursDEFAULT_CANCEL_WINDOW = 5 minutessetCancelWindow(onlyOwner) writescancelWindowimmediately after the 15s–24h check and emitsCancelWindowUpdated. Same pattern forsetFeeParams,addOperator/removeOperator,addCanceler/removeCanceler._authorizeUpgradeis alsoonlyOwnerwith an empty body.PendingWithdrawstoresapprovedAtbut no per-row window._validateWithdrawExecutionandwithdrawCancelboth usew.approvedAt + cancelWindow(the current storage value).Forge currently locks in the 15s floor as valid:
test_SetCancelWindow_MinBoundinBridge.t.solcallssetCancelWindow(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_delayrequiresinfo.sender == config.admin, accepts15..=86400, andWITHDRAW_DELAY.savewith no pending proposal.Cancel and execute load that live item:
PendingWithdrawhasapproved_atonly.DEFAULT_WITHDRAW_DELAY = 300.Admin rotation is already two-step with
ADMIN_TIMELOCK_DURATION = 604_800(propose_admin/accept_admininexecute/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_configmoves admin, operator, fee, delay, and pauseset_config.rs: signer must bebridge.admin. Optional fields includenew_admin,operator,fee_bps,withdraw_delay,paused. Delay must be>= 15 && <= 86400. Admin reassignment is single-step (no pending admin).PendingWithdrawhasapproved_atonly.withdraw_executeusespw.approved_at + bridge.withdraw_delay.setCancelWindow+ execute/cancel; Terraexecute_set_withdraw_delay+ withdraw execute/cancel; Solanaset_configdelay/admin/operator/fee; tests + security-model docs. Not operator M-of-N (#176). Not Terramin_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
(srcChain, nonce)replay. Execute still requiresapprovedafter the window.__gap/ append-only layout so existing pending rows stay loadable. Solana: account resize/migrate must keep live withdraw PDAs loadable. Terra: extendPendingWithdrawwithout invalidating the active-withdraw index._authorizeUpgradeas 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.getCancelWindow/WITHDRAW_DELAYfor UX. After snapshotting, remaining-time for a given hash must use the row window, not only the global config.ready. No public mainnet shrink-window recipe.Relevant files
packages/contracts-evm/src/Bridge.solMIN_CANCEL_WINDOW = 15;setCancelWindowonlyOwner; execute/cancel use livecancelWindowpackages/contracts-evm/src/interfaces/IBridge.solPendingWithdrawhas no snapshotted windowpackages/contracts-evm/test/Bridge.t.soltest_SetCancelWindow_MinBoundasserts 15s is legalpackages/contracts-terraclassic/bridge/src/execute/config.rsexecute_set_withdraw_delay(15–86400)packages/contracts-terraclassic/bridge/src/execute/withdraw.rsWITHDRAW_DELAYpackages/contracts-terraclassic/bridge/src/execute/admin.rspropose_admin/accept_admin(rotation only)packages/contracts-terraclassic/bridge/src/state.rsDEFAULT_WITHDRAW_DELAY = 300; pending hasapproved_atonlypackages/contracts-solana/programs/cl8y-bridge/src/instructions/set_config.rspackages/contracts-solana/programs/cl8y-bridge/src/instructions/withdraw_execute.rsapproved_at + bridge.withdraw_delaypackages/contracts-solana/programs/cl8y-bridge/src/state/pending_withdraw.rsdocs/security-model.mdOpenZeppelin
Ownable2Step.solis already vendored underpackages/contracts-evm/lib/openzeppelin-contracts/(and upgradeable counterparts in the upgradeable package if that is whatBridgeimports). PreferOwnable2StepUpgradeablefor the UUPS proxy.Recommended direction
paused). Flip or rewritetest_SetCancelWindow_MinBoundand the Terra/Solana 15s-accept tests.cancelWindow/withdraw_delayon the pending row. Execute and cancel use that field only.Ownable2StepUpgradeable; Solana pending-admin + accept (mirror Terra). Instantnew_admininset_configgoes away.docs/security-model.mdso “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
setCancelWindow(15)/ Terradelay_seconds = 15/ Solanawithdraw_delay = 15reverts (or is allowed only while paused and cannot be used to execute).approvedAt + W, and cancel remains valid untilapprovedAt + W.transferOwnership/ Solananew_admindoes not complete until the new admin accepts. Terra 7-day admin timelock still holds.Verification (non-exploitative)
Local Forge / CosmWasm / Anchor tests only. Do not publish a mainnet owner call that shortens the window.
setCancelWindow(15)reverts while unpaused;14still reverts; paused-only 15s if that exception is chosen.vm.warpon the timelock, not by skipping it). Execute still reverts untilapprovedAt + 300. Cancel still succeeds inside 300s.execute_set_withdraw_delay+WithdrawCancel/ execute-unlock.set_configdelay 15 reverts while unpaused; in-flight PDA keeps snapshotted delay;new_admindoes not stick until accept.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.