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

Unsafe Token Transfer in Fee Collection

bravejoy780

Root + Impact

Description

  • Contract imports SafeERC20 and uses safeTransferFrom() in buySnow(), but collectFee() uses raw transfer() which can silently fail with non-standard tokens.

// Root cause in the codebase with @> marks to highlight the relevant section
@> using SafeERC20 for IERC20; // Declared but not used consistently
function collectFee() external onlyCollector {
@> i_weth.transfer(s_collector, collection); // Should be safeTransfer
}

Risk

Likelihood: Occurs every time collector withdraws fees; standard WETH works but non-standard tokens or upgrades could break

Impact: Fee transfers could silently fail, permanently trapping protocol revenue in the contract

Proof of Concept

The inconsistency is evident in the codebase. buySnow() properly uses safe transfers while collectFee() does not, indicating developer oversight.


// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
​
// Simulates a token that doesn't return boolean on transfer
contract NonCompliantToken {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
}
function approve(address spender, uint256 amount) external returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) external {
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
// Notice: no return value
}
function transfer(address to, uint256 amount) external {
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
// Notice: no return value
}
}
​
contract UnsafeTransferTest is Test {
function testInconsistentSafetyPattern() public {
// This test documents the code inconsistency:
//
// In buySnow(): safeTransferFrom is used correctly
// In collectFee(): raw transfer is used incorrectly
//
// The fix is straightforward: use safeTransfer in collectFee()
}
}

Recommended Mitigation

Simply replace the raw transfer() call with safeTransfer() to maintain consistency with the rest of the codebase and ensure proper error handling.

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!!!");
+
+ emit FeeCollected();
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 13 days 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!