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

Signers can delay confirmations indefinitely to bypass timelock then execute proposals immediately, leading to governance attack

Author Revealed upon completion

Signers can delay confirmations indefinitely to bypass timelock then execute proposals immediately, leading to governance attack

Description

  • Signers can strategically withhold confirmations until AFTER the timelock period expires, then confirm and execute immediately. This completely defeats the purpose of the timelock as a cooling-off period.

  • The timelock clock should start after sufficient confirmations, not after proposal.
    Currently: \

    • Timelock starts: When transaction is proposed
      - Timelock should start: When quorum is reached

// Root cause in the codebase with @> marks to highlight the relevant section
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
// CHECKS
// 1. Check if enough confirmations
if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
// 2. Check if timelock period has passed
uint256 requiredDelay = _getTimelockDelay(txn.value);
@> uint256 executionTime = txn.proposedAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
if (txn.value > address(this).balance) {
revert MultiSigTimelock__InsufficientBalance(address(this).balance);
}
​
// EFFECTS
// 3. Mark as executed BEFORE the external call (prevent reentrancy)
txn.executed = true;
​
// INTERACTIONS
// 4. Execute the transaction
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) {
revert MultiSigTimelock__ExecutionFailed();
}
​
// 5. Emit event
emit TransactionExecuted(txnId, txn.to, txn.value);
}
​

Risk

Likelihood:

  • HIGH, anytime signers can choose to wait till expiration and confirm the pending proposal later to bypass timelocks.

Impact:

  • HIGH. The timelock becomes completely ineffective against coordinated signers. Malicious actors can:

  • The intended design of waiting period for reconsideration is completely eliminated.

Proof of Concept

  • Deploy the contract with multiple signers.

  • Propose a transaction and do not confirm it.

  • Advance time beyond the timelock period.

  • Have the required signers confirm the transaction.

  • Execute the transaction immediately after confirmations.

  • The transaction executes without any delay, demonstrating that the timelock was effectively bypassed.

    Paste the following test case into test/MultiSigTimelock.t.sol to reproduce:

function testBypassTimelockByDelayingConfirmationsTillExpiry() external {
vm.startPrank(OWNER);
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
multiSigTimelock.grantSigningRole(SIGNER_FOUR);
multiSigTimelock.grantSigningRole(SIGNER_FIVE);
vm.stopPrank();
​
uint256 amountToSend = 100 ether;
vm.deal(address(multiSigTimelock), amountToSend);
vm.deal(OWNER, amountToSend);
vm.prank(OWNER);
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, amountToSend, hex"");
​
vm.expectRevert(); //MultiSigTimelock__InsufficientConfirmations(3,0)
multiSigTimelock.executeTransaction(txnId);
​
uint256 timelockDelay = 7 days;
vm.warp(block.timestamp + timelockDelay);
​
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
​
multiSigTimelock.executeTransaction(txnId);
}

Recommended Mitigation

  • Track Confirmed At timestamp for each transaction by adding this field to the Transaction struct.

  • Start the timelock countdown only after the transaction reaches the required confirmations.

  • Modify executeTransaction to check if the current time is greater than Confirmed At + TIMELOCK_DURATION.

  • This ensures that even if signers delay their confirmations, the timelock period is always enforced after quorum is achieved.

struct Transaction {
address to;
uint256 value;
bytes data;
uint256 confirmations;
uint256 proposedAt;
+ uint256 confirmedAt; // NEW: When quorum was reached
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].confirmedAt = 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);
}
​
+ // NEW: Timelock starts from confirmation, not proposal
uint256 requiredDelay = _getTimelockDelay(txn.value);
- uint256 executionTime = txn.proposedAt + requiredDelay;
+ uint256 executionTime = txn.confirmedAt + requiredDelay; // NOT txn.proposedAt
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
​
+ // ... rest of execution logic
}

Support

FAQs

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

Give us feedback!