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

Only owner can propose a transaction

Author Revealed upon completion

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);
}

Support

FAQs

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

Give us feedback!