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

Owner can DOS by limiting signers to less than required confirmations

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!