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

Owner Has God-Mode Control Over Signers, Defeating Multisig Security

Author Revealed upon completion

Description

The owner can add and remove signers without requiring any multisig approval. This gives the owner complete control over who can approve transactions, allowing them to bypass the entire multisig security model by adding addresses they control.

In standard multisig implementations (like Gnosis Safe), adding or removing signers requires a multisig vote from existing signers. This contract instead gives the owner sole authority:

function grantSigningRole(address _account)
external
nonReentrant
onlyOwner // @> Only owner can add signers, no multisig vote
noneZeroAddress(_account)
{
// ... owner can add ANYONE they want
s_signers[s_signerCount] = _account;
s_isSigner[_account] = true;
s_signerCount += 1;
_grantRole(SIGNING_ROLE, _account);
}
​
function revokeSigningRole(address _account)
external
nonReentrant
onlyOwner // @> Only owner can remove signers, no multisig vote
noneZeroAddress(_account)
{
// ... owner can remove ANYONE they want
}

There is no documentation warning users that the owner has this level of control. Users depositing funds expect a standard multisig where signers are independent and changes require consensus.

Risk

Likelihood:

  • Owner can execute this at any time by controlling the signer set

  • No external conditions required

Impact:

  • Owner can set up "multisig" with only addresses they control

  • Owner can replace legitimate signers with sock puppets after users deposit funds

  • Complete bypass of multisig security - effectively becomes a single-signer wallet

  • 100% fund loss possible

  • Users have false sense of security

Attack Scenarios:

  1. Honeypot Setup: Owner deploys with 3 addresses they control, markets as "secure multisig", users deposit, owner drains

  2. Rug Pull: Owner starts legitimate with real signers, gains trust, removes real signers and adds controlled addresses, steals funds

  3. Hostile Takeover: Owner removes signers who disagree with malicious proposal, adds compliant sock puppets, forces transaction through

Proof of Concept

function test_OwnerCanBypassMultisigWithControlledSigners() public {
// Setup - owner adds 2 addresses they control
address addr1 = makeAddr("controlled1");
address addr2 = makeAddr("controlled2");
multiSigTimelock.grantSigningRole(addr1);
multiSigTimelock.grantSigningRole(addr2);
// Now owner controls all 3 signers
assertEq(multiSigTimelock.getSignerCount(), 3);
// Fund the multisig
vm.deal(address(multiSigTimelock), 100 ether);
address ownerWallet = makeAddr("ownerWallet");
// Owner proposes transaction to send funds to themselves
vm.prank(OWNER);
uint256 txnId = multiSigTimelock.proposeTransaction(ownerWallet, 50 ether, hex"");
// Owner confirms with all their addresses
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(addr1);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(addr2);
multiSigTimelock.confirmTransaction(txnId);
// Wait for timelock and execute
vm.warp(block.timestamp + 2 days + 1);
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txnId);
// Transaction executed - owner got the funds
assertEq(ownerWallet.balance, 50 ether);
}

Run: forge test --mt test_OwnerCanBypassMultisigWithControlledSigners -vv

Recommended Mitigation

Require multisig consensus to add or remove signers, matching the standard multisig implementations:

- function grantSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
+ function grantSigningRole(address _account) external nonReentrant noneZeroAddress(_account) {
+ // Require this to be called via executeTransaction with multisig approval
+ require(msg.sender == address(this), "Must be executed by multisig");
if (s_isSigner[_account]) {
revert MultiSigTimelock__AccountIsAlreadyASigner();
}
if (s_signerCount >= MAX_SIGNER_COUNT) {
revert MultiSigTimelock__MaximumSignersReached();
}
s_signers[s_signerCount] = _account;
s_isSigner[_account] = true;
s_signerCount += 1;
_grantRole(SIGNING_ROLE, _account);
}
​
- function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
+ function revokeSigningRole(address _account) external nonReentrant noneZeroAddress(_account) {
+ // Require this to be called via executeTransaction with multisig approval
+ require(msg.sender == address(this), "Must be executed by multisig");
}

Alternatively, clearly document in NatSpec that the owner has unilateral control over signers and users should verify signer independence before depositing funds.

Support

FAQs

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

Give us feedback!