Submission Details
Impact:
Likelihood:
collectFee() uses an unchecked WETH transfer, so a failed fee collection can pass silently
Root + Impact
Description
// src/Snow.sol - fee collection does not check the WETH transfer result
@> function collectFee() external {
@> if (msg.sender != i_feeCollector) {
@> revert S__NotAllowed();
@> }
@> uint256 wethBalance = i_weth.balanceOf(address(this));
@> if (wethBalance > 0) {
@> i_weth.transfer(i_feeCollector, wethBalance);
@> }
@> (bool success,) = i_feeCollector.call{value: address(this).balance}("");
@> if (!success) {
@> revert S__TransferFailed();
@> }
@> }
i_weth.transfer() returns a bool that is never checked. If the WETH token returns false instead of reverting (as some tokens do), collectFee() completes successfully while the WETH fees stay in the contract, and the collector has no way of knowing the collection failed. Note the asymmetry inside the same function: the ETH leg is checked and reverts with S__TransferFailed, but the WETH leg is not. The contract already imports SafeERC20 and uses safeTransferFrom() in buySnow(), so this is also inconsistent with its own pattern. The standard WETH9 and the project's MockWETH return true, so this is a robustness issue rather than an observed loss - hence Low.
Risk
Likelihood:
Impact:
Proof of Concept
// Explanation: the return value of the WETH transfer is discarded, so a token that returns false instead of reverting makes collectFee() look successful while the fees stay behind.
// 1. Deploy with a WETH token whose transfer() returns false instead of
// reverting (the project's MockWETH returns true, so this needs a
// non-standard token - the point is the missing check, not the mock).
// 2. A user buys Snow with WETH, so the Snow contract holds a WETH balance.
// 3. The fee collector calls collectFee(). i_weth.transfer() returns
// false, the return value is ignored, and the call succeeds.
// 4. The WETH fees remain in the Snow contract and the collector receives
// nothing, with no error raised.
// With the standard WETH9/MockWETH the transfer returns true, so the
// issue is the missing check rather than an observed failure.
Recommended Mitigation
Explanation: use the SafeERC20 wrapper the contract already imports, matching buySnow().
- i_weth.transfer(i_feeCollector, wethBalance);
+ i_weth.safeTransfer(i_feeCollector, wethBalance);
SafeERC20 is already imported and used for safeTransferFrom in buySnow(), so this is a one-line change.
Rewards breakdown
This contest has a flat reward structure with the following payouts:
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.