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

Owner can DOS by limiting signers to less than required confirmations

tanug

Owner can DOS by limiting signers to less than or equal to required confirmations

Description

  • The owner controls adding signers but REQUIRED_CONFIRMATIONS is fixed at 3. The owner can add just 2 additional signers (total 3), making transactions require 3/3 confirmations, allowing any single dissenting signer to block all transactions.

  • Any single signer can block any transaction indefinitely.

// Root cause in the codebase with @> marks to highlight the relevant section
@> function grantSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
if (s_isSigner[_account]) {
revert MultiSigTimelock__AccountIsAlreadyASigner();
}
​
if (s_signerCount >= MAX_SIGNER_COUNT) {
revert MultiSigTimelock__MaximumSignersReached();
}
​
s_signers[s_signerCount] = _account;
s_isSigner[_account] = true;
s_signerCount += 1;
​
_grantRole(SIGNING_ROLE, _account);
}

Risk

Likelihood:

  • Low. To ensure proposals are signed successfully, an owner would typically add more signers to maintain safety

Impact:

  • HIGH. Complete governance paralysis. The owner (can permanently disable the multi-sig functionality by granting role to limited signers.

Proof of Concept

  1. Deploy the contract with only the owner as signer.

  2. Owner adds 2 more signers (total 3).

  3. With only 2 confirmations, no transaction can reach the required 3 confirmations, effectively locking all the funds indefinitely.

    Paste the following test case into test/MultiSigTimelock.t.sol to reproduce:

function testLimitedSignersCanCauseDOS() external {
vm.startPrank(OWNER);
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
vm.stopPrank();
​
// ARRANGE
vm.deal(address(multiSigTimelock), OWNER_BALANCE_ONE);
vm.deal(OWNER, OWNER_BALANCE_ONE);
vm.prank(OWNER);
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, OWNER_BALANCE_ONE, hex"");
​
​
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
​
vm.prank(SIGNER_TWO);
vm.expectRevert();
//confirmations of revoked signers are being considered here as well
multiSigTimelock.executeTransaction(txnId);
}

Recommended Mitigation

  • Make signer addition/removal require multi-sig confirmation

  • Signers should be able to propose adding/removing signers

  • Signers should be penalized for inactivity to prevent long-term DOS

Updates

Lead Judging Commences

kelechikizito Lead Judge 28 days ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!