Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

collectFee() does not check WETH transfer return value — fee collection silently fails

squashingbugsforlife

Root + Impact

Description

  • Normal behavior: collectFee() transfers all accumulated WETH fees from the Snow contract to the collector address.

  • The issue: i_weth.transfer(s_collector, collection) uses the raw IERC20.transfer() whose return value is not checked. Snow.sol already imports and uses SafeERC20 correctly in buySnow() via i_weth.safeTransferFrom(). The inconsistency in collectFee() means a failed WETH transfer goes undetected — the function continues to send native ETH to the collector while WETH remains permanently stuck in the contract.

```solidity
// src/Snow.sol#101-107
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
// @> return value not checked — silent failure possible
// @> inconsistent: buySnow() uses safeTransferFrom() correctly
i_weth.transfer(s_collector, collection);
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
```

Risk

Likelihood:

  • The collector calls this function to withdraw fees — a silent WETH failure means permanent revenue loss.

  • Snow.sol already uses SafeERC20 in buySnow() — the omission in collectFee() is a clear inconsistency that will eventually trigger.

Impact:

  • WETH fees are silently lost — collector receives ETH but not WETH with no error raised.

  • Protocol revenue is permanently locked in the contract with no recovery mechanism.

Proof of Concept

The following Foundry test demonstrates that collectFee() silently succeeds even when the WETH transfer fails. A mock WETH token that returns false on transfer is used to simulate the failure. The function completes without reverting, the collector receives ETH but not WETH, and WETH remains stuck in the contract with no error raised.

```solidity
function testUncheckedTransferSilentFailure() public {
MockWETH mockWeth = new MockWETH();
Snow snow = new Snow(address(mockWeth), buyFee, collector);
​
vm.startPrank(user);
mockWeth.approve(address(snow), buyFee * 10);
snow.buySnow(10);
vm.stopPrank();
​
uint256 wethBefore = mockWeth.balanceOf(address(snow));
assertGt(wethBefore, 0);
​
vm.prank(collector);
snow.collectFee(); // Does NOT revert
​
// WETH still stuck in contract
assertEq(mockWeth.balanceOf(address(snow)), wethBefore);
}
```

Recommended Mitigation

The fix replaces the raw IERC20.transfer() call with SafeERC20.safeTransfer() which is already imported and used correctly elsewhere in Snow.sol. SafeERC20 wraps the transfer and reverts if the return value is false or if the token does not return a value at all. This brings collectFee() into consistency with buySnow() which already uses i_weth.safeTransferFrom() correctly. No new imports are required.

```diff
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
- i_weth.transfer(s_collector, collection);
+ // safeTransfer reverts on failure — consistent with buySnow()
+ i_weth.safeTransfer(s_collector, collection);
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 7 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!