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

This contract allows only the owner to initiate transactions, violating the multi-signature principle.

kevin1115

Root + Impact

Description

  • The onlyOwner restriction limits transactions to the owner (the deployer or a single account subsequently designated), rather than allowing any multisignature member (signer) to submit transactions.e or problem in one or more sentences

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
onlyOwner// The `onlyOwner` restriction limits transactions to the owner (the deployer or a single account subsequently designated),
​
returns (uint256)
{
return _proposeTransaction(to, value, data);
}
​

Risk

Likelihood:

Impact:

  • The onlyOwner restriction limits the right to initiate transactions

  • This violates the core design principles of MultiSig.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "../src/MultiSigTimelock.sol";
​
contract MultiSigTimelockTest is Test {
MultiSigTimelock multiSig;
​
address owner = address(0xA11CE);
address signer1 = address(0xB0B);
address signer2 = address(0xC0C);
address signer3 = address(0xD0D);
address recipient = address(0xE0E);
​
function setUp() public {
// Deploy contract as owner
vm.prank(owner);
multiSig = new MultiSigTimelock();
​
// Add 3 signers (owner is already signer)
vm.prank(owner);
multiSig.grantSigningRole(signer1);
​
vm.prank(owner);
multiSig.grantSigningRole(signer2);
​
vm.prank(owner);
multiSig.grantSigningRole(signer3);
}
​
function testOwnerCanProposeTransaction() public {
// vm.prank() sets msg.sender for the next call
vm.prank(owner);
uint256 txId = multiSig.proposeTransaction(recipient, 1 ether, "");
​
MultiSigTimelock.Transaction memory txn = multiSig.getTransaction(txId);
assertEq(txn.to, recipient);
assertEq(txn.value, 1 ether);
assertEq(txn.executed, false);
}
​
function testSignerCannotProposeTransaction() public {
vm.prank(signer1);
vm.expectRevert("Ownable: caller is not the owner");
multiSig.proposeTransaction(recipient, 1 ether, "");
}
​
function testNonSignerCannotProposeTransaction() public {
address attacker = address(0x999);
vm.prank(attacker);
vm.expectRevert("Ownable: caller is not the owner");
multiSig.proposeTransaction(recipient, 1 ether, "");
}
}

Recommended Mitigation

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
onlyRole(SIGNING_ROLE)
returns (uint256)
{
return _proposeTransaction(to, value, data);
}
​
Updates

Lead Judging Commences

kelechikizito Lead Judge
11 days ago
kelechikizito Lead Judge 4 days ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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

Give us feedback!