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

Revoked signer confirmations still count toward quorum

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: Transactions require REQUIRED_CONFIRMATIONS from current signers.

  • Issue: A signer who confirmed a transaction and was later revoked still counts toward confirmations at execution time.

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// @> relies on stored counter, not on current signer set
if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
}

Risk

Likelihood:

  • Reason 1 // Roles change over time; owners revoke signers for security

  • Reason 2 // Pending transactions may get executed after membership changes

Impact:

  • Impact 1 // Execution may proceed with fewer active signers than quorum

  • Impact 2 // Governance guarantees broken; potential unauthorized asset movement

Proof of Concept

Explanation: The test testH2_RevocationBlocksExecutionWithStaleQuorum shows that if a signer confirms and is then revoked, the transaction can still be executed because the contract only checks the cached confirmations count.

// Setup: 5 signers; propose and collect 3 confirmations
// Owner revokes one of the confirming signers before execution
vm.prank(OWNER);
multiSigTimelock.revokeSigningRole(SIGNER_THREE);
// Execute still succeeds because 'confirmations' counter wasn't revalidated
vm.prank(SIGNER_FOUR);
multiSigTimelock.executeTransaction(txnId);

Recommended Mitigation

Explanation: In _executeTransaction, iterate through the current s_signers array and count how many valid signers have signed the transaction. Use this count instead of txn.confirmations.

- if (txn.confirmations < REQUIRED_CONFIRMATIONS) { revert ... }
+ // Recompute valid confirmations from current signer set before execution
+ uint256 valid = _countValidConfirmations(txnId);
+ if (valid < REQUIRED_CONFIRMATIONS) {
+ revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, valid);
+ }

Status: Valid (Mitigated in src/MultiSigTimelock.sol)

Support

FAQs

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

Give us feedback!