MultiSig Timelock

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

Potential wallet deadlock when signer count < quorum

Author Revealed upon completion

Description

  • A multisig should ensure the active signer count never drops below the quorum required to execute transactions; otherwise, remaining signers can’t reach the threshold and funds become stuck.

  • REQUIRED_CONFIRMATIONS is a fixed constant set to 3, but revokeSigningRole only prevents removing the last signer (s_signerCount <= 1). It allows reducing the signer set to 2. Since executeTransaction requires confirmations >= 3, any wallet with only two active signers is unable to execute any transaction - creating a permanent deadlock unless new signers are added (which may be impossible after other admin mistakes, e.g., owner renounce).

// Fixed quorum = 3
uint256 private constant REQUIRED_CONFIRMATIONS = 3; // @>
// Revoking a signer only blocks when count would drop to <= 1; allowing 2 signers
function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// ...
if (s_signerCount <= 1) { // @> allows reduction to 2 signers
revert MultiSigTimelock__CannotRevokeLastSigner();
}
// ... swap & pop removal, s_signerCount -= 1;
}
// Execution enforces fixed quorum, regardless of current signer count
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
if (txn.confirmations < REQUIRED_CONFIRMATIONS) { // @> hard check: needs 3 confirmations
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
// ...
}

Risk

Likelihood: Low

  • During routine membership changes, an owner revokes a signer and the set drops to two members - perfectly allowed by current checks.

  • This commonly happens when team size shrinks, a signer loses a device, or during incident response when you urgently remove a compromised signer.

Impact: High

  • Permanent execution lock: With only two signers, reaching 3 confirmations is impossible; no transactions can be executed.

  • Operational paralysis: Treasury funds are effectively frozen; if other admin capabilities are lost (e.g., owner renounced), recovery can be impossible.

Proof of Concept

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

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

function testDeadlockWhenSignerCountBelowQuorum() public {
// Owner is signer #1 by default
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
// Propose a 0-ETH transaction to avoid timelock effects
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 0, hex"");
// Reduce signer set back to 2 (allowed by current logic)
multiSigTimelock.revokeSigningRole(SIGNER_THREE);
assertEq(multiSigTimelock.getSignerCount(), 2);
// Both remaining signers confirm
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
// Execution is impossible: requires 3 confirmations but only 2 exist
vm.expectRevert(); // MultiSigTimelock__InsufficientConfirmations(3, 2)
multiSigTimelock.executeTransaction(txnId);
}

Output

[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testDeadlockWhenSignerCountBelowQuorum() (gas: 284920)
Traces:
[361415] MultiSigTimeLockTest::testDeadlockWhenSignerCountBelowQuorum()
├─ [83181] 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]
├─ [83547] MultiSigTimelock::proposeTransaction(spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], 0, 0x)
│ ├─ emit TransactionProposed(transactionId: 0, to: spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], value: 0)
│ └─ ← [Return] 0
├─ [12228] MultiSigTimelock::revokeSigningRole(signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])
│ ├─ emit RoleRevoked(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Stop]
├─ [543] MultiSigTimelock::getSignerCount() [staticcall]
│ └─ ← [Return] 2
├─ [0] VM::assertEq(2, 2) [staticcall]
│ └─ ← [Return]
├─ [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::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [5223] MultiSigTimelock::executeTransaction(0)
│ └─ ← [Revert] MultiSigTimelock__InsufficientConfirmations(3, 2)
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.45ms (510.90µs CPU time)
Ran 1 test suite in 15.10ms (3.45ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Prevent signer count < quorum

  • Consider renaming the error to reflect “cannot reduce below quorum,” e.g., CannotReduceBelowQuorum

function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// ...
- if (s_signerCount <= 1) {
+ // Disallow any action that would reduce signer count below REQUIRED_CONFIRMATIONS
+ if (s_signerCount <= REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__CannotRevokeLastSigner();
}
// ... removal logic
}

Support

FAQs

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

Give us feedback!