MultiSig Timelock

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

No transaction cancellation mechanism

Author Revealed upon completion

Description

  • Multisig systems typically allow a proposed transaction to be canceled/invalidated (e.g., by the proposer or by a quorum of signers) when it’s found to be malicious, outdated, or otherwise undesirable. Canceled transactions should no longer be confirmable or executable.

  • Once a transaction is proposed, there is no cancellation mechanism. The Transaction struct has no canceled flag, there is no cancelTransaction function, and existing flows (confirmTransaction, revokeConfirmation, executeTransaction) do not check for cancellation. The transaction remains in storage indefinitely and can be reconfirmed/executed at any future time if quorum is later met.

struct Transaction {
address to;
uint256 value;
bytes data;
uint256 confirmations;
uint256 proposedAt;
bool executed; // @> no 'canceled' flag
}
function confirmTransaction(uint256 txnId)
external
nonReentrant
transactionExists(txnId)
notExecuted(txnId)
onlyRole(SIGNING_ROLE)
{ // @> no cancellation check here
_confirmTransaction(txnId);
}
function executeTransaction(uint256 txnId)
external
nonReentrant
onlyRole(SIGNING_ROLE)
transactionExists(txnId)
notExecuted(txnId)
{ // @> no cancellation check here either
_executeTransaction(txnId);
}

Risk

Likelihood: Medium

  • During normal operations, teams often discover a proposed transaction is wrong or risky after it’s created. Because there’s no cancel path, that transaction remains actionable forever.

  • Over time (e.g., after membership changes or incident response), new or different signers may accidentally (or intentionally) reconfirm and execute the stale transaction.

Impact: Medium

  • Governance & safety risk: Malicious/outdated proposals can be executed later, even after signers attempted to “abandon” them via revocations.

  • Operational overhead & confusion: Unbounded accumulation of stale proposals increases surface area for mistakes and complicates audits, monitoring, and tooling.

Proof of Concept

  • Copy the code below to MultiSigTimeLockTest.t.sol.

  • Run command forge test --mt testPendingTransactionPersistsWithoutCancellation -vv.

function testPendingTransactionPersistsWithoutCancellation() public {
// Propose a zero-value transaction (no timelock)
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 0, hex"");
console2.log("Transaction ID:", txnId);
// Grant two more signers (owner is already a signer)
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
MultiSigTimelock.Transaction memory txn = multiSigTimelock.getTransaction(txnId);
assertEq(txn.confirmations, 0);
console2.log("Initial number of confirmations:", txn.confirmations);
// Confirm by three signers
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
txn = multiSigTimelock.getTransaction(txnId);
assertEq(txn.confirmations, 3);
console2.log("Number of confirmations after three signers confirmed:", txn.confirmations);
// Revoke all confirmations to simulate an attempt to "cancel"
multiSigTimelock.revokeConfirmation(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.revokeConfirmation(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.revokeConfirmation(txnId);
txn = multiSigTimelock.getTransaction(txnId);
assertEq(txn.confirmations, 0);
console2.log("Number of confirmations after all signers revoked confirmation:", txn.confirmations);
// Later, the same transaction can be re-confirmed again (still valid)
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
// The transaction persists and reaches quorum again — no cancellation available
txn = multiSigTimelock.getTransaction(txnId);
assertEq(txn.confirmations, 3);
console2.log("Number of confirmations after re-confirming:", txn.confirmations);
}

Output:

[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testPendingTransactionPersistsWithoutCancellation() (gas: 424594)
Logs:
Transaction ID: 0
Initial number of confirmations: 0
Number of confirmations after three signers confirmed: 3
Number of confirmations after all signers revoked confirmation: 0
Number of confirmations after re-confirming: 3
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.14ms (491.10µs CPU time)
Ran 1 test suite in 12.21ms (3.14ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Introduce an explicit cancellation state and enforce it across all flows:
    1. Extend the Transaction struct;
    2. Add a notCanceled modifier;
    3. Gate confirmTransaction & executeTransaction with notCanceled;
    4. Provide an admin/quorum-based cancel function.

struct Transaction {
address to;
uint256 value;
bytes data;
uint256 confirmations;
uint256 proposedAt;
bool executed;
+ bool canceled;
}
+modifier notCanceled(uint256 _transactionId) {
+ if (s_transactions[_transactionId].canceled) {
+ revert MultiSigTimelock__TransactionCanceled(_transactionId);
+ }
+ _;
+}
function confirmTransaction(uint256 txnId)
external
nonReentrant
transactionExists(txnId)
notExecuted(txnId)
+ notCanceled(txnId)
onlyRole(SIGNING_ROLE)
{
_confirmTransaction(txnId);
}
function executeTransaction(uint256 txnId)
external
nonReentrant
onlyRole(SIGNING_ROLE)
transactionExists(txnId)
notExecuted(txnId)
+ notCanceled(txnId)
{
_executeTransaction(txnId);
}
+function cancelTransaction(uint256 txnId)
+ external
+ nonReentrant
+ transactionExists(txnId)
+ notExecuted(txnId)
+ onlyOwner // or require N-of-M signers: onlyRole(SIGNING_ROLE) + quorum check
+{
+ s_transactions[txnId].canceled = true;
+ // (Optional) emit TransactionCanceled(txnId);
+}

Support

FAQs

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

Give us feedback!