Root + Impact

• Root: MultiSigTimelock::proposeTransaction only allows the owner of the contract (i.e. first signer) to propose the transactions, and no one else.

• Impact: Due to this, the contract becomes centralised in more than one scenario, literally failing the promise that "any signer can propose transactions" in the docs.

Description

• Normal Behaviour: All users who are granted the SIGNING_ROLE can propose transactions without any restriction.

• Issue: Due to the presence of the onlyOwner modifier in the proposeTransaction function, only the owner of this contract is allowed to propose any new transactions.

Risk

• Likelihood: High

  • It's obvious that any user with SIGNING_ROLE will try to propose transactions, but ends up being disappointed.

• Impact: Medium

  • Provides the Centralisation tag to this protocol

  • Breaks the promise made by the protocol

  • However, there's no financial harm due to this. Just a loss of a feature to signers.

Proof of Concept

• Pls add test__OnlyOwnerCanProposeTransactions test to test/unit/MultiSigTimelockTest.t.sol file

  function test__OnlyOwnerCanProposeTransactions() public grantSigningRoles {

  vm.deal(OWNER, 1 ether);

  ​

  vm.prank(OWNER);

  uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 0.5 ether, hex"");

  console2.log("Transaction Id", txnId);

  ​

  // Signer_two tries to propose a transaction, but fails

  vm.prank(SIGNER_TWO);

  vm.expectRevert(); // Try removing this line!!

  multiSigTimelock.proposeTransaction(SPENDER_ONE, 0.5 ether, hex"");

  ​

  }
  
  

• Run the test using: forge test --mt test__OnlyOwnerCanProposeTransactions -vv

• Here's what the logs show:

  Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest

  [PASS] test__OnlyOwnerCanProposeTransactions() (gas: 432068)

  Logs:

  Transaction Id 0

  ​

  Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.90ms (233.38µs CPU time)

Recommended Mitigation

Simply replace the onlyOwner modifier with onlyRole(SIGNING_ROLE), leading any user with SIGNING_ROLE to propose the transaction.