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

Hardcoded Quorum Exceeds Signer Count via revokeSigningRole Leading to Permanent DoS

Author Revealed upon completion

Root + Impact

Root Cause

The vulnerability is a logic mismatch between the fixed quorum and dynamic membership :

  • Fixed Quorum : REQUIRED_CONFIRMATIONS is hardcoded to 3

  • Insufficient Validation: revokeSigningRole only prevents the signer count from reaching 1, but allows it to drop to 2

  • The Deadlock: Once signers drop below 3, the condition confirmations >= 3 becomes impossible to satisfy, permanently bricking the contract.


Description

  • The MultiSigTimelock contract implements a multi-signature mechanism with a hardcoded quorum of 3 confirmations. However, the revokeSigningRole function lacks a safety check to ensure the total number of signers remains equal to or greater than this required threshold.

  • If an admin revokes a signer and reduces the total count to 2, the contract enters an irreversible "bricked" state. Since no transaction can ever collect 3 signatures, it becomes impossible to execute any further actions—including adding new signers or withdrawing funds—leading to a permanent loss of all assets held in the contract.

// @audit - This check is insufficient as it only prevents count from reaching 0 or 1.
// Since REQUIRED_CONFIRMATIONS is 3, any count below 3 bricks the contract.
​
@> if (s_signerCount <= 1) {
@> revert MultiSigTimelock__CannotRevokeLastSigner();
@> }

Risk

Likelihood:

  • Occurs during routine team management or signer off-boarding processes.

  • Lack of input validation in revokeSigningRole allows human error to override operational requirements.

Impact:

  • Impact 1: Permanent Denial of Service (DoS) the contract becomes unusable for all parties.

  • Impact 2: Irreversible Asset Lock all funds are trapped forever as the quorum can never be reached.

Proof of Concept

function test_BrickContractViaRevoke() public {
// 1. Initially bring the team to 3 members (the minimum threshold for the contract to function)
vm.startPrank(OWNER);
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
vm.stopPrank();
​
assertEq(multiSigTimelock.getSignerCount(), 3);
​
// 2. Admin decides to revoke SIGNER_THREE
// According to current code logic, this is allowed because the count does not drop below 1
vm.prank(OWNER);
multiSigTimelock.revokeSigningRole(SIGNER_THREE);
​
assertEq(multiSigTimelock.getSignerCount(), 2);
console2.log("SignerCount is now 2. Remaining: Owner and Signer_Two");
​
// 3. Attempt to perform a simple transaction
vm.prank(OWNER);
uint256 txId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, "");
​
// Both remaining signers confirm the transaction
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txId);
​
// 4. Attempt to execute (This will fail!)
// Because confirmations (2) are less than REQUIRED_CONFIRMATIONS (3)
vm.expectRevert();
// At this point, MultiSigTimelock__InsufficientConfirmations error should be returned
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txId);
​
console2.log("Bug Confirmed: Contract is bricked. Funds are locked forever!");
}

  1. Add the Test: Copy the provided test_BrickContractViaRevoke function into your test suite (e.g., test/unit/MultiSigTimelockTest.t.sol).

  2. Run the Test: Execute the following command in your terminal:

forge test --mt test_BrickContractViaRevoke -vvvv

Expected Output

git:(main*)$ forge test --mt test_BrickContractViaRevoke -vvv
[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] test_BrickContractViaRevoke() (gas: 316536)
Logs:
SignerCount is now 2. Remaining: Owner and Signer_Two
Bug Confirmed: Contract is bricked. Funds are locked forever!
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.28ms (2.79ms CPU time)
​
Ran 1 test suite in 99.47ms (16.28ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// CHECKS
if (!s_isSigner[_account]) {
revert MultiSigTimelock__AccountIsNotASigner();
}
- // Prevent revoking the first signer (would break the multisig), moreover, the first signer is the owner of the contract(wallet)
- if (s_signerCount <= 1) {
- revert MultiSigTimelock__CannotRevokeLastSigner();
- }
+ // Ensure the number of signers never drops below the required quorum (3)
+ // This prevents a logical deadlock where transactions can no longer be executed.
+ if (s_signerCount <= REQUIRED_CONFIRMATIONS) {
+ revert MultiSigTimelock__InsufficientSignersForQuorum();
+ }
​
// Find the index of the account in the array
// ... rest of the code

Support

FAQs

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

Give us feedback!