MultiSig Timelock

Description / Vulnerability Details:The contract defines a hardcoded constant REQUIRED_CONFIRMATIONS = 3 (Line 86). However, the revokeSigningRole function (Lines 206-247) allows the owner to remove signers as long as the total signer count is greater than 1 (Lines 216-218).This creates a critical logical flaw: If the contract has exactly 3 signers, the owner can revoke one signer. The s_signerCount becomes 2. The revokeSigningRole function does not revert because $2 > 1$.However, any subsequent attempt to execute a transaction via executeTransaction (specifically the internal _executeTransaction) will fail. Line 323 checks:Solidityif (txn.confirmations < REQUIRED_CONFIRMATIONS) { ... }
Since there are only 2 signers remaining, they can never reach the required 3 confirmations.Impact:The contract enters a "bricked" state (Denial of Service). No transactions can be executed, and funds held in the contract are effectively locked. While the owner could theoretically add a new signer to resolve this, if the owner's key is lost or compromised in this state, the funds are lost permanently. This violates the core availability requirement of the multisig wallet.Recommended Mitigation:Update the validation check in revokeSigningRole to ensure the number of signers never drops below the required confirmation threshold.Change Lines 216-218 to:Solidityif (s_signerCount <= REQUIRED_CONFIRMATIONS) {
revert("Cannot reduce signers below required quorum");
}
Title: Timelock mechanism can be bypassed by splitting large transactions into smaller amounts (Structuring)

Description / Vulnerability Details: The MultiSigTimelock contract determines the execution delay (Timelock) based strictly on the value of a single transaction proposal. The logic in _getTimelockDelay (Lines 378-393) returns NO_TIME_DELAY (0 seconds) for any transaction value less than 1 ETH.

There is no mechanism to track the cumulative outflow of funds over time (e.g., a daily limit). This allows a malicious group of signers (or an attacker who compromised the keys) to bypass the 7-day or 1-day waiting periods intended for large withdrawals.

Proof of Concept:

An attacker wants to withdraw 100 ETH immediately (which normally requires a 7-day delay).

Instead of submitting one transaction for 100 ETH, the attacker submits 101 transactions, each for 0.99 ETH.

The _getTimelockDelay function returns 0 for each transaction because 0.99 ETH < 1 ETH.

The attacker executes all 101 transactions in the same block, draining the wallet instantly.

Impact: The security guarantees provided by the Timelock are rendered ineffective. The protocol cannot prevent the rapid draining of large funds, defeating the purpose of the tiered delay system.

Recommended Mitigation: Implement a Rate Limiter or a Daily Spending Limit. Track the total ETH withdrawn within a rolling window (e.g., 24 hours). If the cumulative amount exceeds the threshold (e.g., 1 ETH), enforce the maximum timelock delay on all subsequent transactions, regardless of their individual value.
Title: QA Report: Floating Pragma and Typos reduce code quality

Description: 1. Floating Pragma: The contract uses a floating pragma pragma solidity ^0.8.19;. Locking the pragma helps ensure that contracts do not accidentally get deployed using a compiler version different from the one used during testing and auditing, which could introduce subtle bugs or behavior changes.

2. Typos in Comments: There is a typo in the inline comment within the _executeTransaction function at Line 344:

Solidity

// 5. Emit eventt
"Event" is misspelled as "eventt".

Impact: Low. These issues do not pose an immediate security threat but affect the maintainability, readability, and deterministic deployment of the code.

Recommended Mitigation:

Lock the compiler version: pragma solidity 0.8.19;

Correct the typo to: // 5. Emit event