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

Improper access control in `MultiSigTimeLock::proposeTransaction`

Author Revealed upon completion

Root + Impact

  • Only the owner can propose transactions, preventing signers from performing their intended function

  • Centralizes control, reducing decentralization of the multisig system

  • Breaks the expected behaviour described in the documentation

Description

The proposeTransaction() function incorrectly uses the onlyOwner modifier, allowing only the contract owner to propose transactions.

According to the project's README, any signer should be able to propose a transaction. This creates a logic mismatch between the intended design and the actual implementation.

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

Risk

Likelihood:

  • Anytime a signer tries to propose a transaction, it'll revert

Impact:

  • CentraLized control

Proof of Concept

function testOnlyOwnerCanProposeTranscation()public {
// ARRANGE
address nonOwner = makeAddr("non_owner");
vm.deal(OWNER, OWNER_BALANCE_ONE);
​
// ACT & ASSERT
vm.prank(nonOwner);
vm.expectRevert();
multiSigTimelock.proposeTransaction(SIGNER_TWO, OWNER_BALANCE_ONE, hex"");
}

Recommended Mitigation

Replace the onlyOwner with an onlySigner modifier to align the behaviour with the intended multisig design

+ modifier onlySigner() {
+ if (!s_isSigner[msg.sender]) {
+ revert MultiSigTimelock__CallerNotSigner();
+ }
+ _;
}
​
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
- onlyOwner
+ onlySigner
returns (uint256)
{

Support

FAQs

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

Give us feedback!