MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: high

Timelock Starts at Proposal Time, Allowing Immediate Execution Once Quorum Is Reached After a Long Delay

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: In many timelock-governed multisig workflows, signers expect the timelock delay to provide a predictable “cooldown window” after a transaction is effectively approved (i.e., after reaching the required confirmations), so that execution cannot happen immediately upon the last confirmation.


  • Issue: This implementation anchors the timelock strictly to proposedAt (proposal timestamp), not to the time when quorum is reached. As a result, a transaction can sit unconfirmed for most (or all) of the delay period, and then become executable immediately when the final confirmation is submitted—eliminating the expected post-approval waiting window.


function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
// ...
uint256 requiredDelay = _getTimelockDelay(txn.value);
// @> uint256 executionTime = txn.proposedAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
// ...
}

Risk

Likelihood:

  • In normal usage, proposals can remain pending for extended periods while signers coordinate approvals (hours/days), especially for higher-value transfers that have longer delays.

Impact:

  • The effective timelock protection can be reduced to near-zero at the moment quorum is reached, enabling immediate execution right after the final confirmation.


Proof of Concept

Paste this test into MultiSigTimelock.t.sol. It shows that for a transaction requiring a 1-day timelock (e.g., 5 ETH), if you wait almost 1 day after proposing and only then collect confirmations, execution can succeed immediately after reaching quorum.

function test_TimelockAnchoredToProposedAt_AllowsImmediateExecutionAfterLateQuorum() public grantSigningRoles {
// ARRANGE
// Ensure the wallet has enough balance to execute
vm.deal(address(multiSigTimelock), OWNER_BALANCE_THREE); // 50 ether available
​
// Propose a 5 ETH transaction -> 1 day timelock per _getTimelockDelay
vm.prank(OWNER);
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, OWNER_BALANCE_TWO /* 5 ether */, hex"");
​
// Wait almost the entire timelock period BEFORE collecting confirmations
vm.warp(block.timestamp + 1 days - 1);
​
// Collect quorum confirmations late
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
​
// Move forward by 1 second so proposedAt + 1 day is satisfied
vm.warp(block.timestamp + 1);
​
// ACT: execute immediately after quorum is reached (no additional waiting from quorum time)
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txnId);
​
// ASSERT: funds moved
assertEq(SPENDER_ONE.balance, OWNER_BALANCE_TWO);
}

Recommended Mitigation

Anchor the timelock start to the time quorum is reached (or explicitly document the current semantics). One approach is to store a “quorum reached at” timestamp when confirmations hit REQUIRED_CONFIRMATIONS, and base executionTime on that timestamp.

struct Transaction {
address to;
uint256 value;
bytes data;
uint256 confirmations;
uint256 proposedAt;
+ uint256 quorumAt;
bool executed;
}
​
function _confirmTransaction(uint256 txnId) internal {
if (s_signatures[txnId][msg.sender]) revert MultiSigTimeLock__UserAlreadySigned();
s_signatures[txnId][msg.sender] = true;
​
s_transactions[txnId].confirmations++;
​
+ if (s_transactions[txnId].confirmations == REQUIRED_CONFIRMATIONS) {
+ s_transactions[txnId].quorumAt = block.timestamp;
+ }
​
emit TransactionConfirmed(txnId, msg.sender);
}
​
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
​
uint256 requiredDelay = _getTimelockDelay(txn.value);
- uint256 executionTime = txn.proposedAt + requiredDelay;
+ uint256 executionTime = txn.quorumAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
// ...
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!