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

Centralization Risk: Use of onlyOwner Instead of Role-Based Access Control Undermines Multi-Sig Decentralization

Author Revealed upon completion

Root + Impact

Description

  • The contract suffers from a logic contradiction: it implements AccessControl for decentralized management but restricts critical functions via the onlyOwner modifier. This hardcoded dependency bypasses the intended role-based hierarchy, centralizing all power into a single address.

  • This creates a Single Point of Failure. A single compromised private key (Owner) can bypass the entire multi-sig consensus, rendering the 3-of-N security model useless and placing all funds under the control of a single individual.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
@> onlyOwner
returns (uint256)
{}

Risk

Likelihood:

  • The inclusion of the onlyOwner modifier on core functions creates a permanent architectural vulnerability that exists from the moment of deployment.

  • The current implementation forces a centralized execution flow, making it impossible for the contract to function as a decentralized multi-sig even under normal operating conditions.

Impact:

  • Impact 1 (Total Centralization): The entire 3-of-N security model is invalidated, transforming a secure multi-signature vault into a high-risk single-signature wallet.

  • Impact 2 (Asset Vulnerability): A single point of failure is introduced where a compromised Owner key leads to the immediate and total loss of control over all locked funds, with no way for other signers to intervene.

Proof of Concept

function test_SignersCannotPropose() public {
// 1. Setup: Add a legitimate signer to the system
vm.prank(OWNER);
multiSigTimelock.grantSigningRole(SIGNER_TWO);
​
// Verify the signer actually holds the SIGNING_ROLE
assertTrue(multiSigTimelock.hasRole(multiSigTimelock.getSigningRole(), SIGNER_TWO));
​
// 2. Scenario: SIGNER_TWO attempts to propose a transaction
// In a decentralized Multi-sig, any authorized signer should be able to propose
vm.startPrank(SIGNER_TWO);
// This is expected to revert because the contract incorrectly uses 'onlyOwner'
// instead of 'onlyRole(SIGNING_ROLE)' for proposals.
vm.expectRevert();
multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, "");
vm.stopPrank();
console2.log("Confirmed: Signers are powerless to propose transactions.");
​
// 3. Proof of Centralization: Only the Owner can propose
vm.prank(OWNER);
uint256 txId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, "");
// If the call reaches here, the Owner succeeded while the Signer failed.
assertEq(txId, 0);
console2.log("Confirmed: Only the Owner holds proposal privileges (Access Control Bypass).");
}

  1. Add the Test: Copy the provided test_SignersCannotPropose function into your test suite (e.g., test/unit/MultiSigTimelockTest.t.sol).

  2. Run the Test: Execute the following command in your terminal:

forge test --mt test_SignersCannotPropose -vvvv

Expected Output :

git:(main*)$ forge test --mt test_SignersCannotPropose -vvv
[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] test_SignersCannotPropose() (gas: 215158)
Logs:
Confirmed: Signers are powerless to propose transactions.
Confirmed: Only the Owner holds proposal privileges (Access Control Bypass).
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 17.72ms (2.98ms CPU time)
​
Ran 1 test suite in 102.02ms (17.72ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Replace the onlyOwner modifier with onlyRole(SIGNING_ROLE) in the proposeTransaction function. This change aligns the contract with a decentralized architecture by allowing any authorized signer to propose actions, rather than restricting this power to a single address.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
- onlyOwner
+ onlyRole(SIGNING_ROLE)
returns (uint256)
{

Support

FAQs

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

Give us feedback!