MultiSig Timelock

First Flight #55
Beginner FriendlyWallet
100 EXP
Submission Details
Impact: medium
Likelihood: medium

Balance check only at execution time

Author Revealed upon completion

Description

  • A robust multisig should surface funding issues early, ideally at proposal or when quorum is first reached, so signers don’t wait through timelocks only to hit an avoidable execution failure.

  • The wallet verifies balance only inside executeTransaction. Proposals can be created for arbitrarily large value, gather confirmations, wait out the timelock, and then revert at execution with MultiSigTimelock__InsufficientBalance. This wastes time, invites griefing, and may leave important payouts stuck until someone notices and funds the wallet.

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// ... quorum & timelock checks
// @> Balance is checked only here, at execution time
if (txn.value > address(this).balance) {
revert MultiSigTimelock__InsufficientBalance(address(this).balance);
}
(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: Medium

  • Routine treasury ops often schedule payments before funds arrive (e.g., incoming revenue, bridge delays). With the current design, teams won’t learn the transaction is unfunded until execution time.

  • This occurs whenever proposals are made ahead of deposits or when signers misestimate available balance.

Impact: Medium

  • Operational failure & delay: After quorum and timelock, the transaction reverts; signers must create a new proposal or retry later, compounding delays around payrolls, vendor payments, or incident responses.

  • Griefing vector: An attacker (or careless signer) can create many unfunded proposals to consume signer attention and timelock windows, clogging governance processes.

Proof of Concept

  • Copy the code below to MultiSigTimeLockTest.t.sol.

  • Run command forge test --mt testBalanceCheckedOnlyAtExecutionTime -vvvv.

function testBalanceCheckedOnlyAtExecutionTime() public {
// Wallet has zero ETH at start
assertEq(address(multiSigTimelock).balance, 0);
// Propose sending 10 ETH (will require a 2-day timelock by design)
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 10 ether, hex"");
// Add two more signers and reach quorum
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
// Wait out the 2-day timelock
vm.warp(block.timestamp + 2 days);
// Attempt to execute: reverts because balance is insufficient,
// demonstrating that the only balance check happens at execution time.
vm.expectRevert(); // MultiSigTimelock__InsufficientBalance(address(this).balance)
multiSigTimelock.executeTransaction(txnId);
}

Output:

[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testBalanceCheckedOnlyAtExecutionTime() (gas: 382474)
Traces:
[399274] MultiSigTimeLockTest::testBalanceCheckedOnlyAtExecutionTime()
├─ [0] VM::assertEq(0, 0) [staticcall]
│ └─ ← [Return]
├─ [107447] MultiSigTimelock::proposeTransaction(spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], 10000000000000000000 [1e19], 0x)
│ ├─ emit TransactionProposed(transactionId: 0, to: spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], value: 10000000000000000000 [1e19])
│ └─ ← [Return] 0
├─ [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]
├─ [0] VM::warp(172801 [1.728e5])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [5728] MultiSigTimelock::executeTransaction(0)
│ └─ ← [Revert] MultiSigTimelock__InsufficientBalance(0)
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.59ms (235.20µs CPU time)
Ran 1 test suite in 13.21ms (2.59ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Fail early (proposal or quorum time), and optionally provide a funded‑state guard:

+ // New error to surface funding issues early
+ error MultiSigTimelock__ProposalUnfunded(uint256 required, uint256 available);
// Option A: enforce funding at proposal time (simple & explicit)
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
onlyOwner
returns (uint256)
{
+ // Require wallet to be funded when proposing value transfers.
+ // If you need to schedule future-funded payouts, create a separate API (see Option C).
+ if (value > address(this).balance) {
+ revert MultiSigTimelock__ProposalUnfunded(value, address(this).balance);
+ }
return _proposeTransaction(to, value, data);
}
// Option B: enforce funding when quorum is first reached (less restrictive, better UX)
function _confirmTransaction(uint256 txnId) internal {
if (s_signatures[txnId][msg.sender]) { revert MultiSigTimeLock__UserAlreadySigned(); }
s_signatures[txnId][msg.sender] = true;
s_transactions[txnId].confirmations++;
+ // When quorum is achieved for the first time, ensure the wallet is funded.
+ if (s_transactions[txnId].confirmations >= REQUIRED_CONFIRMATIONS) {
+ if (s_transactions[txnId].value > address(this).balance) {
+ // Undo this confirmation to avoid ghost quorum on unfunded proposals
+ s_signatures[txnId][msg.sender] = false;
+ s_transactions[txnId].confirmations--;
+ revert MultiSigTimelock__ProposalUnfunded(
+ s_transactions[txnId].value,
+ address(this).balance
+ );
+ }
+ }
emit TransactionConfirmed(txnId, msg.sender);
}
// Option C (advanced): support "scheduled" payouts while guarding execution
+ // Add a flag to mark proposals that must be funded before timelock starts.
+ // Combine with the earlier fix 'timelock starts at quorum' to avoid zero-delay post-funding.
+ // Example sketch:
+ // struct Transaction { ... bool requireFundedAtQuorum; uint256 quorumMetAt; }
+ // On confirm: if requireFundedAtQuorum && balance < value => revert.
+ // Else if balance >= value and quorum achieved => set quorumMetAt = block.timestamp.
+ // Execution still retains the existing runtime balance check as a final safety net.
- // Existing execution-time check can stay as a last line of defense
+ // Keep the execution-time balance check as defense-in-depth even with early guards.
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// (quorum & timelock checks)
if (txn.value > address(this).balance) {
revert MultiSigTimelock__InsufficientBalance(address(this).balance);
}
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) { revert MultiSigTimelock__ExecutionFailed(); }
emit TransactionExecuted(txnId, txn.to, txn.value);
}

Support

FAQs

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

Give us feedback!