MultiSig Timelock

First Flight #55

Beginner FriendlyWallet

100 EXP

View all submissions

Previous Next

Submission Details

Impact: high

Likelihood: high

Timelock is tied to proposal time, not final confirmation

Author Revealed upon completion

Description

A timelock should start only after quorum is reached (or at least after the first time quorum is met), ensuring signers have a guaranteed review window between final approval and execution.
The current implementation starts the countdown at proposedAt (the proposal time), not when quorum is reached. If a transaction is proposed and left idle until the delay expires, signers can later accumulate the required confirmations and execute immediately, eliminating the intended review window between approval and execution.

function _executeTransaction(uint256 txnId) internal {

Transaction storage txn = s_transactions[txnId];

// ... quorum check omitted here for brevity

// @> Timelock base is proposal time, not the time quorum is achieved

uint256 requiredDelay = _getTimelockDelay(txn.value);

uint256 executionTime = txn.proposedAt + requiredDelay;

if (block.timestamp < executionTime) {

revert MultiSigTimelock__TimelockHasNotExpired(executionTime);

}

// external call

(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);

if (!success) revert MultiSigTimelock__ExecutionFailed();

emit TransactionExecuted(txnId, txn.to, txn.value);

}

Risk

Likelihood: High

Teams often draft proposals ahead of time (e.g., during planning) and delay approvals. Any proposal created far in advance will auto‑expire its timelock before confirmations are gathered, enabling instant execution once quorum is met.
This naturally occurs in treasuries with scheduled payouts or in incidents where proposals are created early but confirmed later.

Impact: High

Safety window eliminated: Signers have no delay between final approval and execution, defeating timelock guarantees for high‑value actions.
Governance risk: An attacker who can influence timing may propose early, wait out the delay unnoticed, then quickly obtain quorum and drain funds immediately.

Proof of Concept

Copy the code below to MultiSigTimeLockTest.t.sol.
Run command forge test --mt testTimelockStartsAtProposalNotQuorum -vvvv.

function testTimelockStartsAtProposalNotQuorum() public {

// Fund the contract

vm.deal(address(multiSigTimelock), 100 ether);

// Propose a high-value transaction that should have a 7-day timelock

uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 100 ether, hex"");

// Fast-forward time so that the 7-day delay from proposal time has already expired

vm.warp(block.timestamp + 7 days);

// Grant two more signers (owner is already signer #1)

multiSigTimelock.grantSigningRole(SIGNER_TWO);

multiSigTimelock.grantSigningRole(SIGNER_THREE);

// Reach quorum *now* (long after proposal)

multiSigTimelock.confirmTransaction(txnId);

vm.prank(SIGNER_TWO);

multiSigTimelock.confirmTransaction(txnId);

vm.prank(SIGNER_THREE);

multiSigTimelock.confirmTransaction(txnId);

// Because the timelock was anchored to proposal time, execution can happen immediately

multiSigTimelock.executeTransaction(txnId); // succeeds without waiting after quorum

assertTrue(multiSigTimelock.getTransaction(txnId).executed, "Transaction should be marked as executed");

assertTrue(SPENDER_ONE.balance == 100 ether, "Spender should have received the funds");

assertTrue(address(multiSigTimelock).balance == 0 ether, "MultiSigTimelock should have zero balance after execution");

}

Output:

[⠊] Compiling...

No files changed, compilation skipped

Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest

[PASS] testTimelockStartsAtProposalNotQuorum() (gas: 443665)

Traces:

[463265] MultiSigTimeLockTest::testTimelockStartsAtProposalNotQuorum()

├─ [0] VM::deal(MultiSigTimelock: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 100000000000000000000 [1e20])

│ └─ ← [Return]

├─ [107447] MultiSigTimelock::proposeTransaction(spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], 100000000000000000000 [1e20], 0x)

│ ├─ emit TransactionProposed(transactionId: 0, to: spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], value: 100000000000000000000 [1e20])

│ └─ ← [Return] 0

├─ [0] VM::warp(604801 [6.048e5])

│ └─ ← [Return]

├─ [79181] MultiSigTimelock::grantSigningRole(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])

│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])

│ └─ ← [Stop]

├─ [74381] MultiSigTimelock::grantSigningRole(signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])

│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])

│ └─ ← [Stop]

├─ [51353] MultiSigTimelock::confirmTransaction(0)

│ ├─ emit TransactionConfirmed(transactionId: 0, signer: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])

│ └─ ← [Stop]

├─ [0] VM::prank(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])

│ └─ ← [Return]

├─ [29453] MultiSigTimelock::confirmTransaction(0)

│ ├─ emit TransactionConfirmed(transactionId: 0, signer: signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])

│ └─ ← [Stop]

├─ [0] VM::prank(signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])

│ └─ ← [Return]

├─ [29453] MultiSigTimelock::confirmTransaction(0)

│ ├─ emit TransactionConfirmed(transactionId: 0, signer: signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])

│ └─ ← [Stop]

├─ [63283] MultiSigTimelock::executeTransaction(0)

│ ├─ [0] spender_one::fallback{value: 100000000000000000000}()

│ │ └─ ← [Stop]

│ ├─ emit TransactionExecuted(transactionId: 0, to: spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], value: 100000000000000000000 [1e20])

│ └─ ← [Stop]

├─ [3173] MultiSigTimelock::getTransaction(0) [staticcall]

│ └─ ← [Return] Transaction({ to: 0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23, value: 100000000000000000000 [1e20], data: 0x, confirmations: 3, proposedAt: 1, executed: true })

├─ [0] VM::assertTrue(true, "Transaction should be marked as executed") [staticcall]

│ └─ ← [Return]

├─ [0] VM::assertTrue(true, "Spender should have received the funds") [staticcall]

│ └─ ← [Return]

├─ [0] VM::assertTrue(true, "MultiSigTimelock should have zero balance after execution") [staticcall]

│ └─ ← [Return]

└─ ← [Stop]

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.01ms (390.50µs CPU time)

Ran 1 test suite in 14.27ms (3.01ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Anchor the timelock to the first moment quorum is met and reset it if quorum is lost:

// 1) Extend the Transaction struct to track when quorum is reached

struct Transaction {

address to;

uint256 value;

bytes data;

uint256 confirmations;

uint256 proposedAt;

bool executed;

+ uint256 quorumMetAt; // timestamp when REQUIRED_CONFIRMATIONS are first reached

}

// 2) When confirming, set quorumMetAt the first time threshold is reached

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 this is the first time quorum is met, record the timestamp

+ if (s_transactions[txnId].confirmations >= REQUIRED_CONFIRMATIONS &&

+ s_transactions[txnId].quorumMetAt == 0) {

+ s_transactions[txnId].quorumMetAt = block.timestamp;

+ }

emit TransactionConfirmed(txnId, msg.sender);

}

// 3) If a confirmation is revoked and quorum falls below threshold, clear quorumMetAt

function _revokeConfirmation(uint256 txnId) internal {

if (!s_signatures[txnId][msg.sender]) {

revert MultiSigTimeLock__UserHasNotSigned();

}

s_signatures[txnId][msg.sender] = false;

s_transactions[txnId].confirmations--;

+ if (s_transactions[txnId].confirmations < REQUIRED_CONFIRMATIONS) {

+ s_transactions[txnId].quorumMetAt = 0;

+ }

emit TransactionRevoked(txnId, msg.sender);

}

// 4) In execution, base the timelock on quorumMetAt (fallback to proposedAt for legacy)

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 requiredDelay = _getTimelockDelay(txn.value);

+ uint256 baseTime = txn.quorumMetAt == 0 ? txn.proposedAt : txn.quorumMetAt;

+ uint256 executionTime = baseTime + requiredDelay;

if (block.timestamp < executionTime) {

revert MultiSigTimelock__TimelockHasNotExpired(executionTime);

}

(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);

if (!success) revert MultiSigTimelock__ExecutionFailed();

emit TransactionExecuted(txnId, txn.to, txn.value);

}

Rewards breakdown

nSLOC

205

This contest has a flat reward structure with the following payouts:

High

100 EXP

Medium

20 EXP

Low

2 EXP

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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

Give us feedback!