MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: low
Invalid

Owner Self-Revocation Combined with Signer Removal Creates Permanent Fund Lock

rensley

Root + Impact

Description

  • The contract requires exactly 3 confirmations to execute any transaction, but allows the signer count to drop below 3.

  • The owner can revoke signers (including themselves) as long as at least 1 signer remains.

  • If the signer count drops to 2 or fewer, the 3-confirmation quorum becomes mathematically impossible to achieve, permanently locking all funds.

// Root cause in the codebase with @> marks to highlight the relevant section
​
uint256 private constant REQUIRED_CONFIRMATIONS = 3;
​
function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
if (!s_isSigner[_account]) {
revert MultiSigTimelock__AccountIsNotASigner();
}
@> if (s_signerCount <= 1) { // Only prevents revoking the LAST signer
revert MultiSigTimelock__CannotRevokeLastSigner();
}
// Allows reducing to 2 signers, but 3 confirmations still required
}
​

Risk

Likelihood:

  • Reason 1 // Owner accidentally revokes too many signers during key rotation

  • Reason 2 // Malicious owner intentionally bricks the wallet to grief other signers

  • Reason 3 // Owner revokes compromised signers without realizing quorum impact

Impact:

  • All ETH in the contract becomes permanently inaccessible

  • No recovery mechanism exists - funds are locked forever

  • Quorum can never be reached with fewer than 3 signers

Proof of Concept

This test demonstrates a scenario where the owner removes signers during a routine key rotation, inadvertently reducing the signer count below the required quorum. With only 2 signers remaining, both confirm a withdrawal transaction, but execution fails because 3 confirmations are required. The funds become permanently locked with no recovery path.

function test_PermanentFundLockWithInsufficientSigners() public grantSigningRoles {
// Fund contract with 10 ETH
vm.deal(address(multiSigTimelock), 10 ether);
// Owner revokes 3 signers, leaving only 2 (OWNER + SIGNER_TWO)
multiSigTimelock.revokeSigningRole(SIGNER_THREE);
multiSigTimelock.revokeSigningRole(SIGNER_FOUR);
multiSigTimelock.revokeSigningRole(SIGNER_FIVE);
assertEq(multiSigTimelock.getSignerCount(), 2);
// Propose withdrawal
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, "");
// Both remaining signers confirm
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
// Only 2 confirmations possible, but 3 required - FUNDS LOCKED FOREVER
vm.prank(OWNER);
vm.expectRevert(
abi.encodeWithSelector(
MultiSigTimelock.MultiSigTimelock__InsufficientConfirmations.selector,
3, // required
2 // current (maximum possible)
)
);
multiSigTimelock.executeTransaction(txnId);
// Contract still holds funds with no way to withdraw
assertEq(address(multiSigTimelock).balance, 10 ether);
}

Recommended Mitigation

Enforce minimum signer count equal to required confirmations:

function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
if (!s_isSigner[_account]) {
revert MultiSigTimelock__AccountIsNotASigner();
}
- if (s_signerCount <= 1) {
- revert MultiSigTimelock__CannotRevokeLastSigner();
- }
+ if (s_signerCount <= REQUIRED_CONFIRMATIONS) {
+ revert MultiSigTimelock__WouldBreakQuorum();
+ }
// ... rest of function
}
Updates

Lead Judging Commences

kelechikizito Lead Judge 4 days ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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

Give us feedback!