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

Revoked Signers Confirmations Persist, Allowing Quorum Bypass via "Ghost Signatures"

Author Revealed upon completion

Root + Impact

Description

  • The multi-sig mechanism relies on a static counter for confirmations instead of dynamically validating them against the current signer set. This creates a "Ghost Signature" vulnerability in three steps:

  1. Static Accounting: confirmTransaction increments a standalone integer (confirmations) that is decoupled from the signer’s future authorization status.

  2. Stale State: revokeSigningRole removes a signer's role but fails to decrement counters in pending transactions. The revoked signer’s approval remains "locked" as a valid vote.

  3. Unauthorized Execution: executeTransaction only checks if the static counter >= 3. This allows transactions to pass quorum using "ghost" votes from revoked members, bypassing the 3-of-N active signer requirement.

// 1. In _confirmTransaction (State is permanently incremented):
function _confirmTransaction(uint256 txnId) internal {
s_signatures[txnId][msg.sender] = true;
@> s_transactions[txnId].confirmations++; // @> The count is increased without being linked to the signer's future validity
emit TransactionConfirmed(txnId, msg.sender);
}
​
​
​
​
// 2. In revokeSigningRole (Role is removed, but legacy data remains):
function revokeSigningRole(address _account) external onlyOwner {
// ... logic to remove from array ...
s_signerCount -= 1;
s_isSigner[_account] = false;
@> _revokeRole(SIGNING_ROLE, _account); // @> Role is gone, but s_signatures[txnId][_account] is still 'true' and counter is not updated
}
​
​
​
// 3. In _executeTransaction (Stale data is trusted for final decision):
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
// @> ROOT CAUSE: Execution depends solely on a static counter that may include revoked signers
@> if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
// ...
}

Risk

Likelihood:

  • Happens when a signer is revoked during pending transactions.

Impact:

  • Transactions can execute without enough active signers, allowing unauthorized ETH transfers.

Proof of Concept

function test_GhostSignaturePersistence() public {
// 1. Setup: Owner grants signing roles to two additional signers
vm.startPrank(OWNER);
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
vm.stopPrank();
// 2. Fund the MultiSig contract to enable transaction execution
vm.deal(address(multiSigTimelock), 2 ether);
​
// 3. Propose a transaction (1 ether)
vm.prank(OWNER);
uint256 txId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, "");
​
// 4. SIGNER_THREE confirms the transaction before being revoked
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txId);
​
// 5. Owner revokes SIGNER_THREE's signing role
vm.prank(OWNER);
multiSigTimelock.revokeSigningRole(SIGNER_THREE);
​
// 6. Remaining signers confirm (Total confirmations reach 3 including the revoked signer's "Ghost Signature")
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txId);
​
// 7. Fast-forward time to satisfy the timelock requirement
vm.warp(block.timestamp + 1 days + 1 seconds);
​
// 8. Execute the transaction
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txId);
// Final Verification: The transaction succeeds despite one confirmation coming from a revoked entity
MultiSigTimelock.Transaction memory txn = multiSigTimelock.getTransaction(txId);
assertTrue(txn.executed);
assertEq(SPENDER_ONE.balance, 1 ether);
console2.log("Success! The test passed and the vulnerability is proven.");
}

  1. Add the Test: Copy the provided test_GhostSignaturePersistence function into your test suite (e.g., test/unit/MultiSigTimelockTest.t.sol).

  2. Run the Test: Execute the following command in your terminal:

forge test --mt test_GhostSignaturePersistence -vvvv

Recommended Mitigation

  • Execution quorum is derived from the current signer set, not from stale state.

  • Once a signer is revoked, all their historical approvals become invalid instantly.

  • revokeSigningRole no longer needs to handle legacy transaction state.

// 1. Data Structure Fix
- Store confirmation count inside the Transaction struct.
+ Remove the `confirmations` field entirely from the Transaction struct.
​
​
​
// 2. Execution Logic Fix
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
- if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
- revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
- }
+ uint256 validConfirmations;
+ for (uint256 i = 0; i < s_signerCount; i++) {
+ address signer = s_signers[i];
+ if (s_signatures[txnId][signer]) {
+ validConfirmations++;
+ }
+ }
+
+ if (validConfirmations < REQUIRED_CONFIRMATIONS) {
+ revert MultiSigTimelock__InsufficientConfirmations(
+ REQUIRED_CONFIRMATIONS,
+ validConfirmations
+ );
+ }
​
// ... remaining execution logic
}
​
​
// 3. Confirmation Logic Simplification
function _confirmTransaction(uint256 txnId) internal {
- s_transactions[txnId].confirmations++;
}
​
function _revokeConfirmation(uint256 txnId) internal {
- s_transactions[txnId].confirmations--;
}
​

Support

FAQs

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

Give us feedback!