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

Missing Transaction Cancellation Functionality

Author Revealed upon completion

Description:

The contract lacks any mechanism to cancel a proposed transaction. Once a transaction is created via proposeTransaction(), it exists permanently in the contract state and can only be prevented from execution by not gathering enough confirmations or by waiting for the timelock to expire.

However, the timelock only delays execution—it doesn't prevent it. There is no way for signers to formally reject or cancel a malicious or erroneous transaction, even if it gains confirmations. The only defense is for signers to withhold confirmations or revoke them individually, but there's no collective cancellation mechanism.

Impact:

Malicious or erroneous transactions remain in the system indefinitely, creating several risks:

  1. A transaction proposed with wrong parameters cannot be formally cancelled and might accidentally get confirmed and executed later

  2. The transaction ID counter increments forever, potentially leading to gas-intensive iteration over all transactions

  3. The contract state becomes polluted with invalid transactions

  4. If a majority of signers want to reject a transaction, they must coordinate off-chain to ensure it never reaches the threshold, rather than having an on-chain cancellation mechanism

Proof of Concept:

​
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console2} from "forge-std/Test.sol";
import {MultiSigTimelock} from "src/MultiSigTimelock.sol";
​
contract MultiSigTimelockVulnerabilityTests is Test {
MultiSigTimelock multiSigTimelock;
​
address public OWNER = address(this);
address public SIGNER_TWO = makeAddr("signer_two");
address public SIGNER_THREE = makeAddr("signer_three");
address public SIGNER_FOUR = makeAddr("signer_four");
address public MALICIOUS_SIGNER = makeAddr("malicious_signer");
address public RECIPIENT = makeAddr("recipient");
​
function setUp() public {
multiSigTimelock = new MultiSigTimelock();
}
​
modifier grantSigningRoles() {
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
multiSigTimelock.grantSigningRole(SIGNER_FOUR);
_;
}
​
function test_CannotCancelTransaction() public grantSigningRoles {
address wrongRecipient = makeAddr("wrong_recipient");
vm.deal(address(multiSigTimelock), 10 ether);
​
// Owner accidentally proposes to wrong recipient
vm.prank(OWNER);
uint256 txId = multiSigTimelock.proposeTransaction(wrongRecipient, 5 ether, "");
​
// Owner realizes mistake but cannot cancel
// No cancel function exists
​
// All signers confirm by mistake
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txId);
​
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txId);
​
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txId);
​
// Execute after timelock
vm.warp(block.timestamp + 1 days + 1);
​
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txId);
​
// Wrong recipient received the funds - no cancellation was possible
assertEq(wrongRecipient.balance, 5 ether);
}
​
function test_TransactionStateClutter() public {
// ARRANGE
address wrongRecipient = makeAddr("wrong_recipient");
​
// Propose multiple transactions that should be cancelled
vm.startPrank(OWNER);
for(uint256 i = 0; i < 10; i++) {
multiSigTimelock.proposeTransaction(wrongRecipient, 0.1 ether, "");
}
vm.stopPrank();
​
// All transactions remain in state forever
assertEq(multiSigTimelock.getTransactionCount(), 10);
}
}
​

Mitigation:

Add a cancellation mechanism that requires owner approval or a majority of signers:

event TransactionCancelled(uint256 indexed transactionId, address indexed canceller);
​
function cancelTransaction(uint256 txnId)
external
nonReentrant
onlyOwner
transactionExists(txnId)
notExecuted(txnId)
{
Transaction storage txn = s_transactions[txnId];
​
// Mark as executed to prevent future execution
txn.executed = true;
​
// Clear all confirmations
txn.confirmations = 0;
​
emit TransactionCancelled(txnId, msg.sender);
}

Alternatively, implement a multi-sig cancellation requiring multiple signers to vote for cancellation.

Support

FAQs

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

Give us feedback!