MultiSig Timelock

First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Submission Details
Severity: medium
Valid

No Transaction Cancellation Mechanism

Root + Impact

Description

  • Any onlyOwner account calls proposeTransaction() to create a new Transaction struct with executed = false, stored in s_transactions[txnId], and s_transactionCount is incremented.

  • Signers can confirm at any point, and once confirmations >= REQUIRED_CONFIRMATIONS and the timelock passes, any signer may execute the transaction.

The issue:

  • There is no lifecycle control beyond executed. Proposed transactions cannot be canceled, invalidated, or expired, so:

    • Spam proposals accumulate and are never cleared.

    • Old, partially-confirmed transactions stay live and can later be completed by new, uninformed signers.

// STATE
uint256 private s_transactionCount;
mapping(uint256 transactionId => Transaction) private s_transactions;
mapping(uint256 transactionId => mapping(address user => bool)) private s_signatures; // @> confirmations tracking [file:3]
// PROPOSAL CREATION – ONLY GROWS FORWARD
function _proposeTransaction(address _to, uint256 _value, bytes memory _data)
internal
returns (uint256)
{
uint256 transactionId = s_transactionCount;
s_transactions[transactionId] = Transaction({
to: _to,
value: _value,
data: _data,
confirmations: 0,
proposedAt: block.timestamp,
executed: false
});
s_transactionCount += 1; // @> monotonically increasing, no deletion [file:3]
emit TransactionProposed(transactionId, _to, _value);
return transactionId;
}
// @> NO cancelTransaction(), NO expiry, NO flag other than `executed` to invalidate txns [file:3]

Risk

Likelihood:

  • Occurs whenever a temporary/malicious signer spams proposals during their tenure, or when honest signers create outdated proposals that are never intended to execute.

  • Reappears over time as team composition changes, increasing the chance that new signers unknowingly confirm old, malicious or obsolete transactions.

Impact:

  • Unbounded proposal set plus lack of cancellation/expiry gives attackers long-lived opportunities to weaponize stale transactions against future signers and degrades operational safety of the multisig.


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 NoTransactionCancellationTest is Test {
MultiSigTimelock multiSigTimelock;
address owner = makeAddr("owner");
address maliciousOwner = makeAddr("maliciousOwner");
address newSigner = makeAddr("newSigner");
address anotherSigner = makeAddr("anotherSigner");
address attacker = makeAddr("attacker");
function setUp() public {
vm.prank(maliciousOwner);
multiSigTimelock = new MultiSigTimelock();
// Fund contract
vm.deal(address(multiSigTimelock), 10 ether);
}
function testNoTransactionCancellationMaliciousSpam() public {
// ================= SCENARIO 1: MALICIOUS PROPOSAL SPAM =================
console2.log("SCENARIO 1: Malicious owner spams proposals");
// Malicious owner proposes 5 drain transactions
vm.startPrank(maliciousOwner);
uint256[] memory maliciousTxns = new uint256[](5);
for (uint256 i = 0; i < 5; i++) {
maliciousTxns[i] = multiSigTimelock.proposeTransaction(attacker, 1 ether, "");
console2.log("Malicious proposal #", i, ": txnId =", maliciousTxns[i]);
}
vm.stopPrank();
// ================= PROOF: Transactions PERMANENTLY stored =================
assertEq(multiSigTimelock.getTransactionCount(), 5, "5 malicious txns created");
for (uint256 i = 0; i < 5; i++) {
assertFalse(multiSigTimelock.getTransaction(maliciousTxns[i]).executed, "All txns remain executable");
}
console2.log("PROOF: 5 malicious txns stored FOREVER - cannot be deleted");
// ================= SCENARIO 2: DELAYED EXECUTION AFTER OWNERSHIP TRANSFER =================
console2.log("\nSCENARIO 2: Delayed social engineering attack");
// Transfer ownership to innocent new owner
vm.prank(maliciousOwner);
multiSigTimelock.transferOwnership(owner);
// Add new innocent signers
vm.prank(owner);
multiSigTimelock.grantSigningRole(newSigner);
vm.prank(owner);
multiSigTimelock.grantSigningRole(anotherSigner);
console2.log("Malicious owner transferred ownership, NewSigner added");
// Need 3 confirmations total. The malicious owner (who is deployer) already has signing role from constructor
uint256 dangerousTxn = 0; // First malicious txn
// Malicious owner confirms his own proposal
vm.prank(maliciousOwner);
multiSigTimelock.confirmTransaction(dangerousTxn); // 1/3
// New signers unaware of history - innocently confirm
vm.prank(newSigner);
multiSigTimelock.confirmTransaction(dangerousTxn); // 2/3
vm.prank(anotherSigner);
multiSigTimelock.confirmTransaction(dangerousTxn); // 3/3
console2.log("New signers innocently confirm old malicious txn #0");
// Fast forward timelock
vm.warp(block.timestamp + 7 days + 1);
// ================= EXECUTE OLD MALICIOUS TXN =================
uint256 attackerBefore = attacker.balance;
vm.prank(newSigner);
multiSigTimelock.executeTransaction(dangerousTxn);
assertEq(attacker.balance, attackerBefore + 1 ether, "Attacker drains 1 ETH from old txn");
console2.log("ATTACK SUCCEEDS: NewSigner executes 7-day-old malicious txn");
console2.log(" NO CANCELLATION POSSIBLE -> 1 ETH DRAINED");
// ================= PROOF: REMAINING SPAM STILL EXECUTABLE =================
console2.log("\nREMAINING 4 MALICIOUS TXNS STILL EXECUTABLE:");
for (uint256 i = 1; i < 5; i++) {
console2.log(" txnId", maliciousTxns[i], "still live in storage");
}
}
}

RESULT:

forge test --match-test testNoTransactionCancellationMaliciousSpam -vvv
[⠰] Compiling...
No files changed, compilation skipped
Ran 1 test for test/testNoTransactionCancellationMaliciousSpam.sol:NoTransactionCancellationTest
[PASS] testNoTransactionCancellationMaliciousSpam() (gas: 826116)
Logs:
SCENARIO 1: Malicious owner spams proposals
Malicious proposal # 0 : txnId = 0
Malicious proposal # 1 : txnId = 1
Malicious proposal # 2 : txnId = 2
Malicious proposal # 3 : txnId = 3
Malicious proposal # 4 : txnId = 4
PROOF: 5 malicious txns stored FOREVER - cannot be deleted
SCENARIO 2: Delayed social engineering attack
Malicious owner transferred ownership, NewSigner added
New signers innocently confirm old malicious txn #0
ATTACK SUCCEEDS: NewSigner executes 7-day-old malicious txn
NO CANCELLATION POSSIBLE -> 1 ETH DRAINED
REMAINING 4 MALICIOUS TXNS STILL EXECUTABLE:
txnId 1 still live in storage
txnId 2 still live in storage
txnId 3 still live in storage
txnId 4 still live in storage
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 975.56µs (364.69µs CPU time)
Ran 1 test suite in 7.27ms (975.56µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

This adds a governance-controlled cancellation path and optional expiry, preventing stale or spammed proposals from remaining executable indefinitely

// + Define cancellation event
event TransactionCancelled(uint256 indexed transactionId);
// + Add explicit cancellation, restricted to governance (e.g., owner/admin)
function cancelTransaction(uint256 _transactionId)
external
onlyOwner
transactionExists(_transactionId)
notExecuted(_transactionId)
{
// Optional: disallow canceling fully confirmed txns
require(
s_transactions[_transactionId].confirmations < REQUIRED_CONFIRMATIONS,
"MultiSigTimelock: cannot cancel fully confirmed transaction"
);
// Delete core transaction data
delete s_transactions[_transactionId];
// Clear confirmations for all known signers
for (uint256 i = 0; i < MAX_SIGNER_COUNT; i++) {
address signer = s_signers[i];
if (signer != address(0)) {
delete s_signatures[_transactionId][signer];
}
}
emit TransactionCancelled(_transactionId);
}
// + (Optional) Add expiry in Transaction struct and enforce in execute
// struct Transaction {
// address to;
// uint256 value;
// bytes data;
// uint256 confirmations;
// uint256 proposedAt;
// bool executed;
// uint256 expiresAt; // e.g., proposedAt + 30 days
// }
Updates

Lead Judging Commences

kelechikizito Lead Judge 4 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Missing Tx Cancellation Functionality

Support

FAQs

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

Give us feedback!