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

Propose Permission Mismatch

Author Revealed upon completion

Root + Impact

Description

The project README explicitly states: "Propose new transactions (permission is tied to the role, so any signer can propose)". However, the proposeTransaction function is restricted with onlyOwner.

This implies that regular signers (who are not the owner) cannot initiate transactions, only approve them. This creates a single point of failure: if the Owner is incapacitated or loses their key, the wallet becomes frozen (cannot propose new txs), even if a quorum of signers is available. Since Owner also controls role management, if the Owner key is lost, the protocol is dead. If Signers could propose, the wallet could continue to operate for transfers.

/// File: src/MultiSigTimelock.sol:253
onlyOwner

Risk

Likelihood: High (Code explicitly prevents stated functionality).
Impact: High (Functionality mismatch and potential for operational freeze if Owner is lost).

Proof of Concept

  1. Create a signer Alice.

  2. Alice calls proposeTransaction(...).

  3. Transaction reverts with OwnableUnauthorizedAccount.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console} from "forge-std/Test.sol";
import {MultiSigTimelock} from "../../src/MultiSigTimelock.sol";
​
contract TestUnstoppable is Test {
MultiSigTimelock public multiSig;
address public deployer;
address public alice;
address public bob;
address public charlie;
address public david;
​
function setUp() public {
deployer = makeAddr("deployer");
alice = makeAddr("alice");
bob = makeAddr("bob");
charlie = makeAddr("charlie");
david = makeAddr("david");
​
vm.prank(deployer);
multiSig = new MultiSigTimelock();
}
​
function test_UnstoppableTransaction() public {
// Setup 3 signers (Deployer, Alice, Bob)
vm.startPrank(deployer);
multiSig.grantSigningRole(alice);
multiSig.grantSigningRole(bob);
vm.stopPrank();
​
// Propose malicious/bad tx (send 1 ETH to attacker)
address attacker = address(0x666);
vm.prank(deployer);
uint256 txId = multiSig.proposeTransaction(attacker, 1 ether, "");
​
// Confirm (3/3)
vm.prank(deployer); multiSig.confirmTransaction(txId);
vm.prank(alice); multiSig.confirmTransaction(txId);
vm.prank(bob); multiSig.confirmTransaction(txId);
​
// Timelock starts (1 day for 1 ETH)
// Owner realizes "OH NO! Alice and Bob are malicious!"
// Owner revokes them.
vm.startPrank(deployer);
multiSig.revokeSigningRole(alice);
multiSig.revokeSigningRole(bob);
// Owner tries to stop the tx... but cant cancel.
// Owner adds NEW honest signers (Charlie, David) to try and "dilute" or take control.
multiSig.grantSigningRole(charlie);
multiSig.grantSigningRole(david);
vm.stopPrank();
​
// Time passes
vm.warp(block.timestamp + 1 days + 1);
​
// Tx is STILL confirmed (3/3) because revocation didn't clear votes.
// ANYONE with signing role (e.g. Deployer, or Charlie) can trigger it.
// Even if Charlie tries to "clear" it, he can't. He can only execute it.
vm.deal(address(multiSig), 10 ether);
vm.prank(charlie); // Honest signer unsuspecting OR Malicious/Griefing observer
multiSig.executeTransaction(txId);
​
// Funds lost
assertEq(attacker.balance, 1 ether);
}
}
​

Recommended Mitigation

Align the code with the documentation (and standard multisig behavior) by changing the modifier.

- function proposeTransaction(...) external onlyOwner ...
+ function proposeTransaction(...) external onlyRole(SIGNING_ROLE) ...

Support

FAQs

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

Give us feedback!