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

collectFee() Uses Raw transfer() for WETH While SafeERC20 Is Already Imported and Used Elsewhere

wh4t54u53rn4m3

Root + Impact

Description

The collectFee() function transfers WETH to the fee recipient using a raw .transfer() call. However, SafeERC20 is already imported in the contract and used via safeTransferFrom / safeTransfer in other functions, indicating the codebase convention is to use the Safe wrapper.

The raw .transfer() call works correctly for WETH specifically because WETH's transfer() returns a boolean. However, if the fee token were ever swapped for a non-standard ERC20 that omits the return value (a common pattern in some tokens), the call would silently fail or revert with an unhandled return value.

// Root cause: raw .transfer() instead of safeTransfer()
function collectFee() external onlyOwner {
// ...
IERC20(WETH).transfer(feeRecipient, feeAmount); // no return value checked
}
​
// Contrast: other functions in the same contract use SafeERC20
using SafeERC20 for IERC20;
// ...
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);

Risk

Likelihood:

  • This code path only executes when the owner calls collectFee(), limiting exposure to owner-triggered operations.

  • WETH (the actual fee token) is a standard, well-audited token whose transfer() reliably returns true, so the bug is latent rather than actively exploitable.

Impact:

  • If the fee token is ever changed to a non-standard ERC20 (e.g., a token that omits the boolean return), the transfer will silently fail and fees will be lost or stuck.

  • Inconsistent coding style increases the risk of similar oversights in future modifications.

Proof of Concept

The PoC illustrates that a token omitting the boolean return value would cause safeTransfer() to revert with a clear error, while raw .transfer() would either succeed silently (if the token still moves funds) or revert with an unhelpful error. The inconsistency means the contract is one token swap away from a silent fee-loss bug.

+ contract PoC {
+ // Simulates a non-standard ERC20 that omits the return value
+ contract NonStandardToken {
+ mapping(address => uint256) public balanceOf;
+
+ function transfer(address to, uint256 amount) external {
+ // No return value — common in some ERC20 implementations
+ balanceOf[msg.sender] -= amount;
+ balanceOf[to] += amount;
+ }
+ }
+
+ Snow private snow;
+
+ constructor(address target) {
+ snow = Snow(target);
+ }
+
+ function demonstrate() external {
+ // If collectFee() were pointed at NonStandardToken,
+ // the raw .transfer() would not check the (nonexistent) return value.
+ // With safeTransfer(), OpenZeppelin's SafeERC20 would revert
+ // with "ERC20 transfer did not succeed" instead of silently proceeding.
+ }
+ }

Recommended Mitigation

Replacing the raw .transfer() with safeTransfer() (from the already-imported SafeERC20 library) ensures the return value is explicitly checked. This aligns collectFee() with the rest of the contract's conventions and future-proofs the function against non-standard tokens.

function collectFee() external onlyOwner {
// ...
- IERC20(WETH).transfer(feeRecipient, feeAmount);
+ IERC20(WETH).safeTransfer(feeRecipient, feeAmount);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 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!