MultiSig Timelock

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

Owner can revoke signers below threshold, causing Denial of Service (DoS)

Author Revealed upon completion

[M-#] Owner can revoke signers below threshold, deadlocking the MultiSig

Summary

The revokeSigningRole function does not check the resulting signer count against the required quorum, allowing the owner to accidentally or maliciously render the contract unusable.

Description

The revokeSigningRole function is designed to manage the set of authorized signers. However, it only contains a check to ensure that the contract has at least one signer remaining (s_signerCount <= 1). It fails to account for the REQUEST_CONFIRMATIONS (quorum) requirement. If the OWNER revokes a signer such that the total number of signers becomes less than the number of signatures required to execute a transaction, the contract enters a state of functional deadlock.

Risk

Denial of Service (DoS): The primary risk is a total functional deadlock. If the number of signers falls below the required threshold, the contract can no longer execute transactions.
Frozen Assets: Any funds held by the MultiSig are effectively locked until the state is manually repaired by the owner.
Centralization Dependency: While the owner can "fix" this by adding new signers, the contract loses its "Multi-Sig" security property during the deadlock, as it relies entirely on the owner's single key to restore functionality.

Proof of Concept

Initial State: The contract is deployed with 5 signers and a required quorum of 3 confirmations (REQUEST_CONFIRMATIONS=3).
Action: The owner calls revokeSigningRole for two signers.
Resulting State: The s_signerCount is now 3.
Action: The owner calls revokeSigningRole for one more signer.
Vulnerability: The check s_signerCount <= 1 passes (because 3 > 1), and the signer is removed.
Deadlock: The s_signerCount is now 2, but the contract still requires 3 confirmations to execute any transaction. The contract is now "bricked" and unusable.

Recommendations

Update the requirement check in revokeSigningRole() to ensure that the number of signers can never drop below the current threshold required for execution (REQUEST_CONFIRMATIONS).

Optimized Code Fix:

function revokeSigningRole(address _account) external nonReentrant onlyOwner {
//NOTE
uint256 private constant REQUIRED CONFIRMATION = 3; (which represents the minimum number of s_signerCount)
// ... existing checks ...
// OLD INVARIANT CHECK:(okay) But as much as this prevents the Owner from being revoked, it breaks the contract's logic of having minimum of 3 signers needed for a transaction to be executed
if (s_signerCount <= 1) {
revert MultiSigTimelock__CannotRevokeLastSigner();
}
//NEW INVARIANT CHECK:
// Prevents revoking signers if it would make the quorum impossible to reach.
// If REQUIRED_CONFIRMATIONS is 3, we cannot revoke if s_signerCount is 3.
// This then replaces the "s_signerCount <= 1" check with a stricter requirement.
if (s_signerCount <= REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__SignerCountCannotBeLessThanRequiredConfirmations();
}
// ... array removal logic ...

}

Mitigation Explanation:

The original check if (s_signerCount <= 1) only prevented the contract from having zero signers. However, it did not protect the Quorum Invariant.
By replacing that check with if (s_signerCount <= REQUIRED_CONFIRMATIONS), we ensure that the total number of signers can never fall below the threshold required to actually execute a transaction. This "self-healing" logic prevents the Owner from accidentally or maliciously bricking the contract, ensuring that the MultiSig remains functional at all times.

Why this is important

Enforces Invariants: It proves the code is "self-healing" and doesn't rely on the Owner being perfect.
Prevents "Invalid State": A smart contract should never be able to enter a state where its main function is broken.
Clear Errors: Instead of a transaction just failing because it can't get enough signatures, the revoke action fails with a clear explanation of why (it would break the quorum).

Support

FAQs

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

Give us feedback!