MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Unchecked ERC20 transfer return values cause silent fund loss

wanzablock

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • The normal behavior should be: if a token transfer fails, the transaction reverts.

  • Explain the specific issue or problem in one or more sentences

  • The Pot contract uses raw transfer() and transferFrom() calls without checking return values. Many ERC20 tokens (notably USDT, BNB) return false on failure instead of reverting.

// Root cause in the codebase with @> marks to highlight the relevant section
// Pot.sol - Line 64-66
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward); // @audit Return value ignored
}
​
// Pot.sol - Line 55 (in closePot)
i_token.transfer(msg.sender, managerCut); // @audit Return value ignored
​
// Pot.sol - Line 45 (in claimCut)
_transferReward(player, reward); // @audit Calls unchecked transfer

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • High - Occurs when:

    • Using USDT, BNB, or other non-standard tokens (very common)

    • Recipient is blacklisted on token contract

  • Reason 2

Impact:

  • Impact 1

  • Individual claim failure

  • Impact 2

Proof of Concept

// Mock USDT-style token that returns false on failure
contract MockUSDT {
mapping(address => uint256) public balanceOf;
mapping(address => bool) public blacklist;
function transfer(address to, uint256 amount) external returns (bool) {
if (blacklist[to]) return false; // Silent failure
if (balanceOf[msg.sender] < amount) return false;
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
}
​
// Test: USDT blacklisted recipient
function testUncheckedTransferSilentFailure() public {
MockUSDT usdt = new MockUSDT();
// Setup pot with USDT
address[] memory players = new address[](1);
players[0] = alice;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 1000;
pot = new Pot(players, rewards, IERC20(address(usdt)), 1000);
usdt.mint(address(pot), 1000);
// Blacklist Alice
usdt.blacklist(alice, true);
// Alice claims
uint256 aliceBalanceBefore = usdt.balanceOf(alice);
vm.prank(alice);
pot.claimCut(); // Transaction succeeds!
// But Alice received nothing
assertEq(usdt.balanceOf(alice), aliceBalanceBefore); // Still 0
// Alice's claim is marked complete
assertEq(pot.checkCut(alice), 0); // Can't claim again
// Accounting broken
assertEq(pot.getRemainingRewards(), 0); // Says 0 remain
assertEq(usdt.balanceOf(address(pot)), 1000); // But pot still has 1000!
// Alice permanently lost 1,000 USDT
}
​
// Test: Loop continues after failure
function testClosePortLoopSilentFailures() public {
MockUSDT usdt = new MockUSDT();
// 3 claimants
address[] memory players = new address[](3);
players[0] = alice;
players[1] = bob;
players[2] = charlie;
// All claim, then pot closes
// ... setup and claims ...
// Blacklist Bob
usdt.blacklist(bob, true);
// Close pot (distributes to all 3)
pot.closePot();
// Alice received payout ✓
assertGt(usdt.balanceOf(alice), 0);
// Bob received NOTHING (but no revert)
assertEq(usdt.balanceOf(bob), 0); // Silent loss
// Charlie received payout ✓
assertGt(usdt.balanceOf(charlie), 0);
// No way to know Bob's transfer failed
// His funds are stuck in pot forever
}

Recommended Mitigation

- remove this code// Pot.sol
pragma solidity ^0.8.20;
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {Ownable} from "lib/openzeppelin-contracts/contracts/access/Ownable.sol";
+ import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract Pot is Ownable(msg.sender) {
+ using SafeERC20 for IERC20;
function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
+ // Validate balance before state changes
+ require(i_token.balanceOf(address(this)) >= reward, "Insufficient balance");
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
- _transferReward(player, reward);
+ i_token.safeTransfer(player, reward); // Will revert on failure
}
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
- i_token.transfer(msg.sender, managerCut);
+ i_token.safeTransfer(msg.sender, managerCut); // Will revert on failure
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
+ i_token.safeTransfer(claimants[i], claimantCut); // Will revert on failure
}
}
}
- function _transferReward(address player, uint256 reward) internal {
- i_token.transfer(player, reward);
- }
}
+ add this code
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!