MultiSig Timelock

First Flight #55
Beginner FriendlyWallet
100 EXP
Submission Details
Impact: high
Likelihood: high

`proposeTransaction` uses `onlyOwner` instead of `onlyRole(SIGNING_ROLE)`, preventing signers from proposing

Author Revealed upon completion

proposeTransaction uses onlyOwner instead of onlyRole(SIGNING_ROLE), preventing signers from proposing

Description

According to the README documentation, any signer should be able to propose transactions: "Propose new transactions (permission is tied to the role, so any signer can propose)".

The proposeTransaction function incorrectly uses the onlyOwner modifier instead of onlyRole(SIGNING_ROLE), restricting proposal rights to only the owner.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
@> onlyOwner // Should be onlyRole(SIGNING_ROLE)
returns (uint256)
{
return _proposeTransaction(to, value, data);
}

Risk

Likelihood: High

  • This occurs every time a non-owner signer attempts to propose a transaction

Impact: High

  • Only the contract owner can propose transactions, not all signers as documented

  • Severely limits the functionality of the multi-sig wallet

  • Violates the principle that signers should have equal power once the role is granted

Proof of Concept

Explanation: A signer (SIGNER_TWO) who has been granted SIGNING_ROLE attempts to propose a transaction. Despite having the correct role according to documentation, the call reverts because onlyOwner modifier only allows the contract owner to propose.

function test_SignersCannotPropose() public {
// Grant signing role to SIGNER_TWO
multiSigTimelock.grantSigningRole(SIGNER_TWO);
assertTrue(multiSigTimelock.hasRole(multiSigTimelock.getSigningRole(), SIGNER_TWO));
// SIGNER_TWO tries to propose - should work per README but fails
vm.prank(SIGNER_TWO);
vm.expectRevert();
multiSigTimelock.proposeTransaction(makeAddr("recipient"), 1 ether, "");
}

Expected: Any address with SIGNING_ROLE can propose transactions.
Actual: Only owner can propose; signers get OwnableUnauthorizedAccount error.

Recommended Mitigation

Explanation: Replace onlyOwner with onlyRole(SIGNING_ROLE) to match the documented behavior. This ensures all signers have equal proposal rights as intended by the multi-sig design.

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

Support

FAQs

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

Give us feedback!