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

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

alexscherbatyuk

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)
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!