MultiSig Timelock

First Flight #55
Beginner FriendlyWallet
100 EXP
Submission Details
Impact: high
Likelihood: high

Revoked Signers’ Historical Confirmations Remain Valid

Author Revealed upon completion

Root + Impact

Description

  • When a signer confirms a transaction, their confirmation is recorded and permanently contributes to the transaction’s confirmation count.
    If that signer is later revoked, the contract removes their signing privileges but does not invalidate or discount their historical confirmations.

    As a result, a transaction may still reach execution using confirmations from accounts that are no longer authorized signers, violating the expected governance trust model.

// @> Confirmation count is stored independently of active signer status
s_signatures[txnId][msg.sender] = true;
s_transactions[txnId].confirmations++;

Risk

Likelihood:

  • Signers are commonly revoked due to compromise or governance decisions.

  • Historical confirmations persist automatically without additional attacker action.

Impact:

  • Revoked or compromised signers can continue influencing transaction execution.

  • Governance decisions may be executed based on invalid or outdated authority.

Proof of Concept

function test_HistoricalSignature_RemainsValid_AfterSignerRevoked() public {
uint256 txId = timelock.proposeTransaction(address(0xBEEF), 1 ether, "");
vm.prank(signer1);
timelock.confirmTransaction(txId);
// Owner revokes signer1 after confirmation
timelock.revokeSigningRole(signer1);
vm.prank(signer2);
timelock.confirmTransaction(txId);
vm.prank(signer3);
timelock.confirmTransaction(txId);
vm.warp(block.timestamp + 1 days);
timelock.executeTransaction(txId);
}

Explanation:
This PoC shows that a signer’s confirmation continues to be counted even after their signing role has been revoked. The transaction is executed despite relying on an authorization that should no longer be valid.

Recommended Mitigation

function _executeTransaction(uint256 txnId) internal {
- if (txn.confirmations < REQUIRED_CONFIRMATIONS) revert(...);
+ uint256 validConfirmations = 0;
+ for (uint256 i = 0; i < s_signerCount; i++) {
+ address signer = s_signers[i];
+ if (s_signatures[txnId][signer]) {
+ validConfirmations++;
+ }
+ }
+ if (validConfirmations < REQUIRED_CONFIRMATIONS) revert(...);
}

Explanation:
Rather than relying on a cached confirmation counter, execution should dynamically compute confirmations based only on currently active signers. This ensures that revoked signers lose all governance influence immediately and prevents stale confirmations from affecting transaction execution.

Support

FAQs

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

Give us feedback!