proposeTransactionis restricted to onlyOwner, blocking signers from making transactions proposals.
Description
According to the project documentation (README.md), any address holding the SIGNING_ROLE is expected to be able to propose new transactions in the multisig wallet.
In the actual implementation, the proposeTransaction function is restricted by the onlyOwner modifier. As a result, signers who hold the SIGNING_ROLE but are not the contract owner are unable to propose transactions. This creates a mismatch between the documented behavior and the on-chain logic, potentially misleading users and integrators.
### Signers (holders of SIGNING_ROLE)
- Up to 5 addresses in total (owner + up to 4 others) that possess the `SIGNING_ROLE`.
- **Powers**:
- Confirm pending transaction proposals
- Revoke their own previous confirmation (useful if they change their mind before execution)
- Execute a transaction once:
- At least 3 distinct signers have confirmed, and
- The value-based timelock period has fully elapsed
@> - Propose new transactions (permission is tied to the role, so any signer can propose)
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
@> onlyOwner
returns (uint256)
{
return _proposeTransaction(to, value, data);
}
Risk
Likelihood:
Impact:
-
Signers cannot perform their intended role of proposing new transactions, limiting the operational functionnality of the multisig.
-
The restriction increses centralization risk and may cause delays in transaction execution, as all proposals require onwner intervention.
-
Impact 2
Proof of Concept
The following test demonstrates that an address holding the SIGNING_ROLE cannot propose a transaction due to the onlyOwner restriction.
function testProposeWithSignerRole() public{
multiSigTimelock.grantSigningRole(SIGNER_TWO);
address to = SPENDER_ONE;
uint256 value = 1 ether;
bytes memory data = "";
vm.startPrank(SIGNER_TWO);
vm.expectRevert();
multiSigTimelock.proposeTransaction(to, value, data);
}
Then run :
forge test --mt testProposeWithSignerRole -vvvv
The following output should be optained :
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testProposeWithSignerRole() (gas: 99796)
Traces:
[2405280] MultiSigTimeLockTest::setUp()
├─ [2364461] → new MultiSigTimelock@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
│ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return] 11123 bytes of code
└─ ← [Stop]
[102596] MultiSigTimeLockTest::testProposeWithSignerRole()
├─ [83181] MultiSigTimelock::grantSigningRole(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ ├─ storage changes:
│ │ @ 4: 0 → 0x000000000000000000000000884d93ff97a786c23d91993e20382fc4c26bc2aa
│ │ @ 0x1b38d2010ad28ced672ce9dd1d9d2aba655d886d685d6ee276a49d66aedfac52: 0 → 1
│ │ @ 2: 1 → 2
│ │ @ 0x7ed2ae3fcc403bba9120be8a557eac30c02697b9ffb5faa455162a427324855b: 0 → 1
│ └─ ← [Stop]
├─ [0] VM::startPrank(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [4789] MultiSigTimelock::proposeTransaction(spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], 1000000000000000000 [1e18], 0x)
│ ├─ storage changes:
│ │ @ 0x9b779b17422d0df92223018b32b4d1fa46e071723d6817e2486d003becc55f00: 1 → 2
│ └─ ← [Revert] OwnableUnauthorizedAccount(0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa)
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.66ms (2.15ms CPU time)
Ran 1 test suite in 25.85ms (6.66ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
The call reverts with OwnableUnauthorizedAccount, confirming that signers cannot propose transactions despite holding the SIGNING_ROLE.
Recommended Mitigation
Align the implementation with the documented behavior by allowing any signer to propose transactions. This can be achieved by replacing onlyOwner with onlyRole(SIGNING_ROLE). The owner already holds this role, ensuring backward compatibility.
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
- onlyOwner
+ onlyRole(SIGNING_ROLE)
returns (uint256)
{
return _proposeTransaction(to, value, data);
}
