Root + Impact

Description

• Describe the normal behavior in one or more sentences

Normal behavior:
The collectFee() function is intended to allow the s_collector address to retrieve all accumulated WETH and ETH from the contract in two steps — a standard ERC20 transfer and a low-level ETH transfer using .call.

• Explain the specific issue or problem in one or more sentences

This function assumes that the s_collector is always a safe and trustworthy address. However, since it can be any arbitrary address, a malicious contract set as s_collector could exploit the ETH .call by reverting in its fallback function or consuming excessive gas. This causes the entire collectFee() call to revert, effectively locking up ETH in the contract permanently and denying the real collector their funds.

Likelihood:

• The s_collector address — which has the authority to receive collected fees — can be set to any arbitrary address, including a malicious contract.

• A determined attacker could gain control of the collector role via:

  • Social engineering the project team,

  • Exploiting governance or misconfigured upgrade logic,

  • Front-running a role change transaction before the team sets the intended address.

• Once set, a malicious collector can exploit the contract’s logic to break future fee withdrawals.

Impact:

• The ETH portion of collected protocol fees can be permanently locked inside the contract if the s_collector rejects or reverts the ETH transfer.

• This results in a Denial of Service (DoS) against ETH fee collection, affecting protocol profitability.

• If collectFee() is used in automated bots or integrations (e.g. fee routers, keepers, treasury managers), this failure could cause wider operational disruptions, such as:

  • Broken payment routing,

  • Missed harvests or yield claims,

  • Failing transactions that rely on fee withdrawal completion.

Proof of Concept

Forge Test (Simplified PoC)

CopyEdit

contract MaliciousCollector { fallback() external payable { // Revert on ETH receipt to break collectFee() revert("No ETH for you"); } } function test_MaliciousCollectorBreaksFeeCollection() public { // Owner sets attacker contract as collector MaliciousCollector attacker = new MaliciousCollector(); snow.changeCollector(address(attacker)); // Send ETH to Snow contract vm.deal(address(snow), 1 ether); // Attempt to collect fees -> should revert on fallback vm.expectRevert("Fee collection failed!!!"); snow.collectFee(); }

Recommended Mitigation

Add a try-catch or decouple ETH transfer:

CopyEdit

function collectFee() external onlyCollector { uint256 collection = i_weth.balanceOf(address(this)); i_weth.transfer(s_collector, collection); // Use a low-level .call inside try/catch to prevent ETH lock (bool collected, ) = payable(s_collector).call{value: address(this).balance}(""); if (!collected) { emit FeeCollected(); // Log but don’t revert return; } }

Or separate WETH and ETH collections into two functions:

CopyEdit

function collectWETH() external onlyCollector { uint256 amount = i_weth.balanceOf(address(this)); i_weth.transfer(s_collector, amount); } function collectETH() external onlyCollector { (bool collected, ) = payable(s_collector).call{value: address(this).balance}(""); require(collected, "ETH transfer failed"); }