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

Revoked Signer Confirmations Remain Counted in Pending Transactions

Author Revealed upon completion

ROOT

Impact

Description

  • When a signer is revoked using revokeSigningRole, any confirmations previously provided by that signer on pending transactions remain counted, allowing removed signers to continue influencing transaction execution.

    This breaks the core multisig security assumption that only current signers’ approvals are valid.

/// @> Root cause: signer revocation does not invalidate existing confirmations
function revokeSigningRole(address _account) external {
...
s_isSigner[_account] = false;
_revokeRole(SIGNING_ROLE, _account);
// @> Missing logic to remove confirmations from pending transactions
}

Risk

Likelihood:

  • Revoking signers is part of normal multisig governance and can occur during signer rotation or key compromise recovery.

    Pending transactions commonly exist while signer changes are being made.

Impact:

  • Removed signers retain effective voting power on pending transactions.

  • Transactions may be executed without the required number of confirmations from current signers, breaking multisig security guarantees.

Proof of Concept

// Signers: A1, B2, C3, D4
// REQUIRED_CONFIRMATIONS = 3
​
// 1. Owner proposes a transaction
proposeTransaction(to, value, data);
​
// 2. Signers A1, B2, C3 confirm
confirmTransaction(txId); // A1
confirmTransaction(txId); // B2
confirmTransaction(txId); // C3
​
// confirmations = 3
​
// 3. Owner revokes signer C3
revokeSigningRole(C3);
​
// 4. Transaction is still executable
executeTransaction(txId); // succeeds although C3 is no longer a signer
​
Explanation:
This PoC shows that a signer can be revoked after confirming a transaction, yet their confirmation is still counted. As a result, the transaction remains executable even though the revoked signer is no longer part of the multisig.
​

Recommended Mitigation

// Track transactions signed by each signer
+ mapping(address => uint256[]) private s_signedTxs;
​
function _confirmTransaction(uint256 txnId) internal {
...
+ s_signedTxs[msg.sender].push(txnId);
}
​
function revokeSigningRole(address _account) external {
...
+ uint256[] storage txnIds = s_signedTxs[_account];
+ for (uint256 i = 0; i < txnIds.length; i++) {
+ uint256 id = txnIds[i];
+ Transaction storage txn = s_transactions[id];
+ if (!txn.executed && s_signatures[id][_account]) {
+ s_signatures[id][_account] = false;
+ txn.confirmations--;
+ }
+ }
+ delete s_signedTxs[_account];
+
+ s_isSigner[_account] = false;
}

Support

FAQs

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

Give us feedback!