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

Regular Signers Cannot Propose Transactions Due to `onlyOwner` Restriction

Author Revealed upon completion

Regular Signers Cannot Propose Transactions Due to onlyOwner Restriction

Description

The contract logic for proposing transactions uses the onlyOwner modifier, restricting proposal initiation solely to the contract administrator. This directly violates the project design, stating that all signers should be able to propose transactions, centralizing the proposal workflow to a single account.

The MultiSigTimelock contract is designed to allow all addresses holding the SIGNING_ROLE to participate in the entire lifecycle of a transaction (propose, confirm, execute). However, the external facing proposeTransaction function enforces an overly strict access control check:

249: function proposeTransaction(address to, uint256 value, bytes calldata data)
250: external
251: nonReentrant
252: noneZeroAddress(to)
253: onlyOwner
254: returns (uint256)
255: {
256: return _proposeTransaction(to, value, data);
257: }

By including onlyOwner at [MultiSigTimelock.sol:253], only the contract deployer/administrator (who holds the Ownable role) can call this function. Other signers, added via grantSigningRole and possessing only the SIGNING_ROLE, are excluded from proposing new transactions. This undermines the decentralized nature of a multi-signature wallet proposal mechanism and violates the Signer Capabilities.

Risk

Likelihood: High

  • It centralizes the most critical step of the workflow (initiation) to a single key.

  • If the Owner key is lost or compromised, the entire system enters a deadlock where no new transactions can ever be processed, essentially strictly bricking the contract's utility, even if 4 other verified signers are active and ready.

Impact: High

  • The issue is present in the deployed code logic and will trigger 100% of the time a non-owner signer attempts to propose a transaction. There are no complex preconditions other than "be a signer who is not the owner".

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console2} from "forge-std/Test.sol";
import {MultiSigTimelock} from "src/MultiSigTimelock.sol";
​
contract ProjectRuleViolationPoC is Test {
MultiSigTimelock multiSigTimelock;
address public OWNER = address(this);
address public SIGNER_TWO = makeAddr("signer_two");
address public RECIPIENT = makeAddr("recipient");
​
function setUp() public {
multiSigTimelock = new MultiSigTimelock();
​
multiSigTimelock.grantSigningRole(SIGNER_TWO);
}
​
function test_SignerCannotProposeTransaction() public {
assertTrue(
multiSigTimelock.hasRole(
multiSigTimelock.getSigningRole(),
SIGNER_TWO
)
);
​
bool isSigner = false;
address[5] memory signers = multiSigTimelock.getSigners();
for (uint i = 0; i < 5; i++) {
if (signers[i] == SIGNER_TWO) {
isSigner = true;
break;
}
}
assertTrue(isSigner, "SIGNER_TWO should be in the signers list");
​
// Attempt to propose transaction as SIGNER_TWO
vm.prank(SIGNER_TWO);
​
vm.expectRevert(
abi.encodeWithSelector(
0x118cdaa7, // selector for OwnableUnauthorizedAccount(address)
SIGNER_TWO
)
);
​
multiSigTimelock.proposeTransaction(RECIPIENT, 1 ether, hex"");
}
}

Recommended Mitigation

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