MultiSig Timelock

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

Only owner can propose a transaction

Only owner can propose a transaction.

Description

The proposeTransaction entrypoint is restricted to the contract owner, but the design requires accounts with the signing role to be able to propose transactions. As implemented, signers who are not the owner cannot propose transactions and calls revert with the Ownable.OwnableUnauthorizedAccount(address) custom error.

Risk

Likelihood: High
The issue occurs whenever a signer tries to propose a transaction.

Impact: High
Signers cannot act as transaction proposers, this functionality is available only to owner.

Proof of Concept

Run the following unit test which demonstrates the revert when a signer (not owner) calls proposeTransaction:

function test_proposeTransaction_revertsFoSignerCaller() public {
address recipient = address(1);
uint256 value = 1 ether;
bytes memory data = abi.encodePacked(bytes4(keccak256("drain()")));
vm.prank(OWNER);
wallet.grantSigningRole(SIGNER_TWO);
vm.prank(SIGNER_TWO);
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, SIGNER_TWO));
wallet.proposeTransaction(recipient, value, data);
}

Mitigation

The following code changes suggests replacing the onlyOwner modifier with the role based onlyRole modifier to allow singers proposing new transactions.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
- onlyOwner
+ onlyRole(SIGNING_ROLE)
returns (uint256)
{
return _proposeTransaction(to, value, data);
}
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!