MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Revoked Signer’s Confirmation Remains Valid for Transaction Execution

mafa

Revoked Signer’s Confirmation Remains Valid for Transaction Execution

Description

The contract fails to invalidate existing confirmations when a user's signingRole is revoked. Due to the timelock, a proposal remains pending long enough for a signer's status to change. Consequently, a transaction can still reach the required threshold and be executed using approvals from individuals who no longer hold administrative privileges. This creates a state inconsistency where revoked members still influence outcomes during the delay period.

Risk

Likelihood:

This is a realistic risk when a signer is revoked for dishonest or malicious behavior, as their existing confirmations remain active and can still be used to reach the execution threshold for pending transactions.

Impact:

The multisig consensus is compromised. Transactions can be executed using "stale" approvals from untrusted parties, allowing a revoked signer to still reach the quorum and authorize fund transfers or critical parameter changes.

Proof of Concept

place the following code in MultiSigTimelockTest.t.sol:

function testRevokeSigningRoleDoesNotAffectExistingConfirmations() grantSigningRoles public {
vm.deal(address(multiSigTimelock), 100 ether);
address recipient = makeAddr("recipient");
// propose a transaction
vm.prank(OWNER);
uint256 trx_Id=multiSigTimelock.proposeTransaction(recipient,100 ether,"");
// 3 signers confirm the transaction
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(trx_Id);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(trx_Id);
vm.prank(SIGNER_FOUR);
multiSigTimelock.confirmTransaction(trx_Id);
// revoke the SigningRole from SIGNER_TWO
vm.prank(OWNER);
multiSigTimelock.revokeSigningRole(SIGNER_TWO);
vm.warp(block.timestamp + 7 days);
vm.prank(SIGNER_FOUR);
multiSigTimelock.executeTransaction(trx_Id);
// Transaction go through
MultiSigTimelock.Transaction memory trx = multiSigTimelock.getTransaction(trx_Id);
assert(trx.confirmations >= 3);
assert(trx.executed);
}
​

Recommended Mitigation

The _executeTransaction function should verify that all confirmations originate from accounts that currently hold the signingRole at the time of execution.

function _executeTransaction(uint256 txnId) internal {
- if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
- revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
- }
​
+ uint256 validConfirmations = 0;
+ for (uint256 i = 0; i < s_signers.length; i++) {
+ address signer = s_signers[i];
+ // Check if the address is still an active signer AND previously confirmed this txn
+ if (hasRole(SIGNING_ROLE, signer) && s_signatures[txnId][signer]) {
+ validConfirmations++;
+ }
+ }
+ if (validConfirmations < REQUIRED_CONFIRMATIONS) {
+ revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);}
​
}
Updates

Lead Judging Commences

kelechikizito Lead Judge 4 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Stale Confirmation Vulnerability/Ghost Voting Issue

Support

FAQs

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

Give us feedback!