MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: high
Invalid

Signers cannot create new proposals, only owner can.

silkgut

Root + Impact

Description

  • proposeTransaction function allows to propose new transactions that can be confirmed later by signers to reach at least 3 confirmations to be executed in some delay related to it's value.

  • Only owner can call proposeTransactions due to onlyOwnermodifier in the function. This breaks the rule stated in the Actors section "Propose new transactions (permission is tied to the role, so any signer can propose)"

/**
* @dev External function to propose a transaction
* @param to The address to which the transaction is proposed
* @param value The amount of ETH to be sent
* @param data The data payload of the transaction if initiated by a smart contract
* @return transactionId The ID of the proposed transaction
*/
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
@> onlyOwner
returns (uint256)
{
return _proposeTransaction(to, value, data);
}

Risk

Likelihood:

  • Only occurs when owner proposes new transactions, no one else can.

Impact:

  • Breaks the rule

  • Only owner can create new proposals where owner can also remove other signers and add their own fake signers to confirm transactions.

Proof of Concept

This test will fail on purpose to show the vulnerability.

Encountered 1 failing test in test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[FAIL: OwnableUnauthorizedAccount(0xC5DD14faf50A1b629A785179e61758dCC300c01F)] testSignerCannotProposeTransaction() (gas: 24662)
function testSignerCannotProposeTransaction() public {
vm.deal(SIGNER_FOUR, OWNER_BALANCE_ONE);
​
vm.prank(SIGNER_FOUR);
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, OWNER_BALANCE_ONE, hex"");
console2.log("This is the first transaction ID", txnId);
assertEq(txnId, 0);
}

Recommended Mitigation

This can be fixed simply adding correct modifier to function.

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!