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

Snow::collectFee uses raw transfer instead of safeTransfer for WETH — silent failure risk on non-reverting tokens

brizybryan

Root + Impact

Description

  • The collectFee function uses i_weth.transfer() to send WETH fees to the collector. The rest of the contract consistently uses SafeERC20 wrappers (e.g., safeTransferFrom in buySnow). Per the ERC20 standard, transfer is allowed to return false on failure instead of reverting. If the WETH token implementation returns false, the transfer silently fails, but collectFee continues to execute the native ETH transfer, emitting no error.

// Root cause in Snow.sol lines 101-107
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection); // @> Raw transfer, not safeTransfer
// @> If transfer returns false, execution continues silently
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
​

Risk

Likelihood:

  • Standard WETH implementations revert on failure, but any non-standard ERC20 token used as WETH (or a future token migration) may return false instead.

  • The contract already imports and uses SafeERC20 for IERC20 — this function simply fails to use the safe wrapper consistently.

Impact:

  • WETH fees are silently lost — the collector believes the fee was collected but the tokens remain in the contract.

  • Inconsistency with the rest of the codebase's safety patterns introduces maintenance risk and breaks the principle of defense-in-depth.

Proof of Concept

This test uses a mock ERC20 that returns false on transfer instead of reverting (which is valid per the ERC20 spec). When collectFee calls i_weth.transfer(...), the return value is ignored and execution continues as if the transfer succeeded. The WETH remains in the Snow contract while the collector receives only the native ETH portion.

// Mock that returns false instead of reverting
contract FalseReturnToken is ERC20 {
constructor() ERC20("BadWETH", "BWETH") {}
function mint(address to, uint256 amount) external { _mint(to, amount); }
function transfer(address, uint256) public pure override returns (bool) {
return false; // ERC20-compliant: returns false on failure
}
}
​
function testM03_UnsafeTransferSilentlyFails() public {
// Deploy Snow with a false-returning token as WETH
FalseReturnToken badWeth = new FalseReturnToken();
address collector = makeAddr("collector");
Snow snowWithBadWeth = new Snow(address(badWeth), 5, collector);
​
// Simulate WETH fees in the contract
badWeth.mint(address(snowWithBadWeth), 10 ether);
assert(badWeth.balanceOf(address(snowWithBadWeth)) == 10 ether);
​
// Collector calls collectFee — transfer returns false but does NOT revert
vm.prank(collector);
snowWithBadWeth.collectFee();
​
// WETH is STILL in the Snow contract — transfer silently failed
assert(badWeth.balanceOf(address(snowWithBadWeth)) == 10 ether);
assert(badWeth.balanceOf(collector) == 0); // Collector got nothing
}
​

Recommended Mitigation

Replace transfer with safeTransfer to match the safety pattern used everywhere else in the contract. The SafeERC20 library is already imported and applied to IERC20, so this is a one-word change that ensures the contract handles non-reverting ERC20 tokens safely.

function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
- i_weth.transfer(s_collector, collection);
+ 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 4 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!