Root + Impact

Description

• Normal Behavior: When the collectFee function is called, the contract should transfer all accumulated WETH fees to the s_collector address and ensure the transaction was successful before proceeding with any state changes.

• Specific Issue: The contract executes i_weth.transfer(s_collector, collection) but does not check the boolean return value. Many ERC20 implementations (including some versions of WETH on different chains) return false instead of reverting if a transfer fails. This leads to a "Silent Failure" where the contract assumes the fees were sent when they were not.

Risk

Likelihood

• Reason 1: Certain ERC20 token implementations are known for returning false on failure (e.g., USDT or older WETH versions on specific sidechains), which is a common pitfall in smart contract development.

• Reason 2: If the s_collector address is a contract that is blacklisted (in certain token types) or cannot receive tokens due to logic errors, the transfer will fail silently.

Impact:

• Impact 1 (Financial Loss): The protocol loses its accumulated fees because the contract logic proceeds as if the funds were successfully moved to the collector.

• Impact 2 (Accounting Errors): Internal accounting or events emitted after the call will report a successful collection, creating a discrepancy between the on-chain data and the actual balance in the collector's wallet.

Proof of Concept

Scenario: The Snow contract has accumulated 10 WETH in fees.

• Trigger: An admin calls collectFee().

• Failure: Due to a non-standard WETH implementation or an issue at the recipient's end, the transfer call fails and returns false.

• Silent Continuation: Because there is no require() or check on the return value, the EVM does not revert the transaction.

• Outcome: The collectFee() execution completes "successfully" in the eyes of the blockchain, but the 10 WETH remains in the Snow contract, and the s_collector receives nothing.

Recommended Mitigation

Implementation

The most secure way to handle ERC20 transfers is using OpenZeppelin's SafeERC20 library, or at minimum, wrapping the call in a require statement.

Explanation
By checking the return value, we ensure that the entire transaction reverts if the transfer does not succeed. This guarantees atomicity: either the collector gets the money and the state updates, or nothing happens at all. This prevents the protocol from losing track of its funds.