MultiSig Timelock

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

Stale Confirmation from Revoked Signer Enables Delayed Execution of Malicious Transaction

Author Revealed upon completion

Root + Impact

Description

  • When a signer is revoked via revokeSigningRole(), their existing confirmations persist in s_signatures and continue counting toward the quorum.

  • Combined with no transaction expiration, a "zombie" confirmation from a revoked signer can be leveraged later to execute an abandoned malicious transaction.

// Root cause in the codebase with @> marks to highlight the relevant section
function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// ... validation ...
s_signers[s_signerCount - 1] = address(0);
s_signerCount -= 1;
s_isSigner[_account] = false;
_revokeRole(SIGNING_ROLE, _account);
@> // MISSING: No cleanup of s_signatures[txnId][_account] for any pending transaction
@> // MISSING: No decrement of s_transactions[txnId].confirmations
}
// The confirmations counter is a cached value that becomes stale when signer status changes
struct Transaction {
// ...
@> uint256 confirmations; // Cached count, never revalidated against current signers
// ...
}

Risk

Likelihood (Low - requires multiple conditions):

  • Reason 1 // Malicious transaction proposed and partially confirmed before compromise detection

  • Reason 2 // Compromised signer revoked but transaction remains pending (no cancel function)

  • Reason 3 // Social engineering needed to obtain remaining confirmations months later

Impact: (High)

  • Theft of contract funds via dormant transaction with zombie confirmation

  • Attack persists indefinitely - no expiration mechanism

Proof of Concept

Simulates a multi-phase attack: compromise detection, signer revocation, time passage, and eventual exploitation when the dormant transaction is forgotten.

function test_ZombieSignerDelayedExecution() public grantSigningRoles {
address ATTACKER_WALLET = makeAddr("attacker_wallet");
// === PHASE 1: Initial Setup (Day 0) ===
// Contract holds significant funds
vm.deal(address(multiSigTimelock), 50 ether);
// Owner proposes legitimate-looking transaction (actually to attacker)
uint256 maliciousTxnId = multiSigTimelock.proposeTransaction(
ATTACKER_WALLET,
0.5 ether, // Small amount to avoid suspicion
""
);
// === PHASE 2: Compromised Signer Confirms (Day 1) ===
// SIGNER_TWO (compromised) confirms the malicious transaction
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(maliciousTxnId);
// Confirmation recorded
assertEq(multiSigTimelock.getTransaction(maliciousTxnId).confirmations, 1);
// === PHASE 3: Compromise Detected, Signer Revoked (Day 2) ===
// Security team detects SIGNER_TWO compromise and revokes immediately
multiSigTimelock.revokeSigningRole(SIGNER_TWO);
// SIGNER_TWO no longer has any role
assertFalse(multiSigTimelock.hasRole(multiSigTimelock.getSigningRole(), SIGNER_TWO));
// CRITICAL BUG: Confirmation count unchanged!
assertEq(multiSigTimelock.getTransaction(maliciousTxnId).confirmations, 1);
// === PHASE 4: Business Continues, New Signer Added (Day 30) ===
address NEW_SIGNER = makeAddr("new_signer");
multiSigTimelock.grantSigningRole(NEW_SIGNER);
// Many other transactions processed, maliciousTxnId forgotten
vm.warp(block.timestamp + 180 days);
// === PHASE 5: Social Engineering Attack (Day 180) ===
// Attacker contacts current signers: "Hey, there's an old pending
// transaction for 0.5 ETH to cover some past expense. Can you approve?"
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(maliciousTxnId);
vm.prank(SIGNER_FOUR);
multiSigTimelock.confirmTransaction(maliciousTxnId);
// === PHASE 6: Execution with Zombie Confirmation ===
// Quorum = 3 (1 zombie + 2 legitimate)
assertEq(multiSigTimelock.getTransaction(maliciousTxnId).confirmations, 3);
uint256 attackerBalanceBefore = ATTACKER_WALLET.balance;
vm.prank(SIGNER_THREE);
multiSigTimelock.executeTransaction(maliciousTxnId);
// Attacker received funds using 6-month-old zombie confirmation
assertEq(ATTACKER_WALLET.balance - attackerBalanceBefore, 0.5 ether);
}

Recommended Mitigation

Implement confirmation invalidation when revoking signers, and add transaction expiration:

+ error MultiSigTimelock__TransactionExpired();
+ uint256 private constant TRANSACTION_VALIDITY_PERIOD = 30 days;
function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// ... existing checks ...
+ // Invalidate all confirmations from revoked signer
+ for (uint256 i = 0; i < s_transactionCount; i++) {
+ if (!s_transactions[i].executed && s_signatures[i][_account]) {
+ s_signatures[i][_account] = false;
+ s_transactions[i].confirmations--;
+ }
+ }
// ... rest of revocation logic ...
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
+ // Check expiration
+ if (block.timestamp > txn.proposedAt + TRANSACTION_VALIDITY_PERIOD) {
+ revert MultiSigTimelock__TransactionExpired();
+ }
// ... rest of execution logic ...
}

Support

FAQs

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

Give us feedback!