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

Only Owner Can Propose Transactions Instead of All Signers

Author Revealed upon completion

Root + Impact

Description

The proposeTransaction function incorrectly uses onlyOwner instead of onlyRole(SIGNING_ROLE), contradicting the specification that any signer should be able to propose transactions (README.md:50).

This creates centralization where only the owner can propose, while other signer functions (confirmTransaction, revokeConfirmation, executeTransaction) correctly use onlyRole(SIGNING_ROLE).

// Root cause in the codebase with @ marks to highlight the relevant section
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
onlyOwner // @audit Should be onlyRole(SIGNING_ROLE) per specification
returns (uint256)
{
return _proposeTransaction(to, value, data);
}

Comparison with other signer functions:

  • confirmTransaction correctly uses onlyRole(SIGNING_ROLE) (line 268)

  • revokeConfirmation correctly uses onlyRole(SIGNING_ROLE) (line 282)

  • executeTransaction correctly uses onlyRole(SIGNING_ROLE) (line 294)

This inconsistency confirms that proposeTransaction should also use onlyRole(SIGNING_ROLE).

Risk

Likelihood:

  • High: Occurs whenever a non-owner signer attempts to propose a transaction.

Impact:

  • Centralization risk: Single point of failure for proposals

  • Functionality mismatch: Non-owner signers cannot propose despite having SIGNING_ROLE

Proof Of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {MultiSigTimelock} from "src/MultiSigTimelock.sol";
​
contract PoC_OnlyOwnerCanPropose is Test {
MultiSigTimelock public multiSig;
address owner = address(this);
address signerTwo = makeAddr("signerTwo");
address recipient = makeAddr("recipient");
​
function setUp() public {
multiSig = new MultiSigTimelock();
multiSig.grantSigningRole(signerTwo);
}
​
function test_SignerCannotPropose() public {
// Verify signerTwo has SIGNING_ROLE
bytes32 signingRole = multiSig.getSigningRole();
assertTrue(multiSig.hasRole(signingRole, signerTwo));
// Attempt to propose as signerTwo (non-owner)
vm.prank(signerTwo);
vm.expectRevert(); // Reverts: "Ownable: caller is not the owner"
multiSig.proposeTransaction(recipient, 1 ether, "");
}
​
function test_Inconsistency() public {
// Owner can propose
uint256 txnId = multiSig.proposeTransaction(recipient, 1 ether, "");
// SignerTwo CAN confirm (works correctly)
vm.prank(signerTwo);
multiSig.confirmTransaction(txnId);
// But signerTwo CANNOT propose (demonstrates bug)
vm.prank(signerTwo);
vm.expectRevert();
multiSig.proposeTransaction(recipient, 1 ether, "");
}
}

Steps to Reproduce:

  1. Deploy MultiSigTimelock contract

  1. Grant SIGNING_ROLE to a non-owner address (e.g., signerTwo)

  1. Verify the address has SIGNING_ROLE using hasRole(SIGNING_ROLE, signerTwo)

  1. Attempt to call proposeTransaction as signerTwo

  1. Transaction reverts due to onlyOwner modifier

  1. Only the owner can successfully propose transactions

Recommended Mitigation

Change the proposeTransaction function to use onlyRole(SIGNING_ROLE) instead of onlyOwner:

This change:

  • Aligns with the contest specification (README.md:50)

  • Matches the pattern used in other signer functions (confirmTransaction, revokeConfirmation, executeTransaction)

  • Restores the intended role-based access control where all signers have equal proposal capabilities

  • Maintains security since only addresses with SIGNING_ROLE can propose (which is managed by the owner)

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!