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

The `MultiSigTimeLock::proposeTransaction` function has the `onlyOwner` modifier, breaking the protocol's core intended functionality.

Author Revealed upon completion

Root + Impact

Description

According to the documentation in README.md, any signer can propose new transactions (permission is tied to the role, so any signer can propose). However, the proposeTransaction() function has the onlyOwner modifier, allowing transaction proposals by the owner only. This severely breaks the protocol's intended functionality, limiting signers to confirmation and execution of owner-proposed transactions.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
@> onlyOwner
returns (uint256)

Risk

Likelihood:

  • Does not require any specific flow, it is a direct limitation of intended functionality.

  • The power to propose transactions is tied to the SIGNER_ROLE according to the documentation in README.md.

Impact:

  • Limits the transaction proposal functionality to the owner only, severely breaking the core functionality.

  • Implements an additional restriction, converting the multisig's role-based functionality to merely confirming and executing the owner's transactions.

Proof of Concept

Add this snipped of code to the MultiSigTimelockTest.t.sol test file.

function test_onlyOwnerCanProposeNewTransaction() public grantSigningRoles {
vm.deal(address(multiSigTimelock), OWNER_BALANCE_TWO);
vm.prank(SIGNER_TWO);
vm.expectRevert(abi.encodeWithSignature("OwnableUnauthorizedAccount(address)", SIGNER_TWO));
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, OWNER_BALANCE_TWO, hex"");
}

How to execute:

forge test --mt test_onlyOwnerCanProposeNewTransaction -vvvv

Recommended Mitigation

Replacing the onlyOwner modifier with onlyRole(SIGNING_ROLE) to allow transaction proposition to any signer.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
- onlyOwner
+ onlyRole(SIGNING_ROLE)
returns (uint256)

Support

FAQs

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

Give us feedback!