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

`collectFee()` uses raw `IERC20.transfer()` instead of `safeTransfer()` leaving return value unchecked

virgilbb

Root + Impact

Description

  • collectFee() calls raw IERC20.transfer() without checking the return value, inconsistent with the rest of the contract which uses SafeERC20.safeTransferFrom(). For any non-standard ERC20 that signals failure by returning false rather than reverting, fee collection completes without error while accumulated WETH fees remain locked in the contract indefinitely.

function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection); // raw transfer, return value ignored
// ...
}

Risk

Likelihood:

  • This only activates when i_weth is a non-standard ERC20 that returns false on failure rather than reverting — standard WETH implementations are not affected under normal conditions.

  • Upgradeable token proxies or future token migrations represent realistic scenarios where this path becomes exploitable. Standard WETH reverts on a failed transfer rather than returning false, making the realistic impact near-zero for this specific deployment, but any token migration or proxy upgrade that changes this behavior would silently break fee collection.

Impact:

  • Fee collection silently fails for non-standard ERC20 tokens — the collector would believe fees were collected when they were not, as the function returns no error.

  • WETH fees accumulate in the contract indefinitely with no recovery path if the token returns false on failure.

Proof of Concept

Place this test in test/ and run forge test --match-test testSilentTransferFailure.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {Snow} from "../src/Snow.sol";
​
contract TestUncheckedTransfer is Test {
Snow public snow;
​
address public mockWeth = makeAddr("weth");
address public collector = makeAddr("collector");
​
uint256 constant BUY_FEE = 5; // Snow constructor multiplies by 1e18
uint256 constant ACCUMULATED_FEES = 1_000e18;
​
function setUp() public {
// Deploy Snow with the mock WETH address.
// Constructor: Snow(address _weth, uint256 _buyFee, address _collector)
// No calls to mockWeth happen during construction, so no mocks needed here.
snow = new Snow(mockWeth, BUY_FEE, collector);
}
​
/// @notice Proves that collectFee() silently ignores a false return value
/// from IERC20.transfer(), leaving the collector with nothing despite the
/// call completing without revert.
function testSilentTransferFailure() public {
// Mock balanceOf(address(snow)) to return ACCUMULATED_FEES,
// simulating WETH fees that have built up in the contract.
vm.mockCall(
mockWeth,
abi.encodeWithSelector(bytes4(keccak256("balanceOf(address)")), address(snow)),
abi.encode(ACCUMULATED_FEES)
);
​
// Mock transfer(collector, ACCUMULATED_FEES) to return false without reverting.
// Snow.collectFee() calls i_weth.transfer() directly (not safeTransfer) and
// never checks the boolean return value — this is the bug being demonstrated.
vm.mockCall(
mockWeth,
abi.encodeWithSelector(
bytes4(keccak256("transfer(address,uint256)")),
collector,
ACCUMULATED_FEES
),
abi.encode(false)
);
​
// Mock balanceOf(collector) to return 0, reflecting that no tokens arrived.
vm.mockCall(
mockWeth,
abi.encodeWithSelector(bytes4(keccak256("balanceOf(address)")), collector),
abi.encode(uint256(0))
);
​
// collectFee() must not revert even though transfer() returned false.
vm.prank(collector);
snow.collectFee();
​
// The collector's balance is still zero — the transfer silently failed.
uint256 collectorBalance = IERC20(mockWeth).balanceOf(collector);
assertEq(
collectorBalance,
0,
"Silent failure: collector balance is still 0 despite collectFee() succeeding"
);
}
}

vm.mockCall makes transfer() return false without reverting. collectFee() completes without error but the collector's balance stays zero — confirming the return value is never checked.

Recommended Mitigation

Replace the raw transfer() call with safeTransfer() from OpenZeppelin's SafeERC20 library so a failed transfer always reverts rather than silently returning false.

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 2 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!