Submission Details
Severity:
Removed Signer's Confirmations Still Count
Root + Impact
`MultiSigTimelock::revokeSigningRole() updates `s_isSigner` and `s_signers[]`, and revokes the signer’s AccessControl role. It does not iterate through pending transactions to clear old confirmations or decrement the confirmation counter.
Transactions may execute under the impression that quorum is met, but quorum includes votes from revoked signers.
Description
When a signer is revoked using `MultiSigTimelock::revokeSigningRole`, their prior confirmations remain stored in `s_signatures[txnId][signer]` and counted in `s_transactions[txnId].confirmations`. This means transactions can appear to have quorum even though one or more confirmations were cast/approved by an entity/signer that is no longer authorized.
Risk
Proof of Concept
Add below code into MultiSigTimelockTest.t.sol file.
function testRevokedSignerStillCountsConfirmation() public {
// ARRANGE
// Grant all remaining signing roles so we have 5 signers in total
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
multiSigTimelock.grantSigningRole(SIGNER_FOUR);
multiSigTimelock.grantSigningRole(SIGNER_FIVE);
// ACT
// OWNER proposes a transaction
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, "");
// Three signers confirm (OWNER, SIGNER_TWO, SIGNER_THREE)
multiSigTimelock.confirmTransaction(txnId); // OWNER
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
// ASSERT before revocation
MultiSigTimelock.Transaction memory txnBefore = multiSigTimelock.getTransaction(txnId);
assertEq(txnBefore.confirmations, 3, "Should have 3 confirmations before revocation");
// ACT: OWNER revokes SIGNER_THREE
multiSigTimelock.revokeSigningRole(SIGNER_THREE);
// ASSERT after revocation
MultiSigTimelock.Transaction memory txnAfter = multiSigTimelock.getTransaction(txnId);
assertEq(txnAfter.confirmations, 3, "Revoked signer still counted in confirmations");
}
Recommended Mitigation
In _executeTransaction(), recompute confirmations by iterating current signers and counting only those with s_signatures[txnId][signer] == true.
Require this recomputed count ≥ REQUIRED_CONFIRMATIONS.
- remove this code
+ add this code
Updates
Lead Judging Commences
kelechikizito 28 days ago
Submission Judgement Published Assigned finding tags:
Stale Confirmation Vulnerability/Ghost Voting Issue
Rewards breakdown
nSLOC
205This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.