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

Unchecked ERC20 Return Values in Pot Transfers — Silent Reward Loss on Non-Reverting Tokens

kivanov

Unchecked ERC20 Return Values in Pot Transfers — Silent Reward Loss on Non-Reverting Tokens

Description

The Pot contract uses bare IERC20.transfer() calls in three critical locations: the manager cut payout in closePot(), the claimant redistribution loop in closePot(), and the reward payout in claimCut() via _transferReward(). None of these calls check the boolean return value specified by the ERC20 standard. While OpenZeppelin's ERC20 implementation reverts on failure, the contract accepts an arbitrary IERC20 token parameter at deployment — meaning any ERC20-compliant token that returns false instead of reverting (e.g., USDT, BNB, OMG) will silently fail.

When a non-reverting token is used, the contract's internal accounting (remainingRewards, playersToRewards) updates as if the transfer succeeded, but no tokens actually move. For claimCut(), the player's reward mapping is zeroed and remainingRewards decremented — the player receives nothing but cannot claim again. For closePot(), the manager cut and redistribution both silently fail, leaving tokens permanently locked in the contract with no recovery mechanism.

// Pot.sol — closePot()
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); // @> return value not checked — silent loss
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut); // @> calls unchecked transfer in loop
}
}
}
​
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward); // @> return value not checked — silent loss
}

Risk

Likelihood

Medium

Impact

High

Severity

Medium

Proof of Concept

A contest is created using a non-reverting ERC20 token (one that returns false on insufficient allowance rather than reverting). Player A calls claimCut() — the internal state updates (playersToRewards[A] = 0, remainingRewards -= reward) but transfer() returns false and the token balance does not change. Player A has been marked as paid but received nothing, and cannot call claimCut() again because playersToRewards[A] is now zero.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test} from "forge-std/Test.sol";
import {Pot} from "../src/Pot.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
contract NonRevertingToken is IERC20 {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
uint8 public decimals = 18;
string public name = "FalseToken";
string public symbol = "FTK";
uint256 public totalSupply;
​
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
totalSupply += amount;
}
​
function transfer(address to, uint256 amount) external returns (bool) {
if (balanceOf[msg.sender] < amount) return false; // silent fail
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
​
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
if (balanceOf[from] < amount) return false;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
​
function approve(address, uint256) external pure returns (bool) { return true; }
}
​
contract UncheckedTransferTest is Test {
function test_silentTransferFail() public {
NonRevertingToken token = new NonRevertingToken();
address player = address(0xBEEF);
address[] memory players = new address[](1);
players[0] = player;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 100 ether;
​
Pot pot = new Pot(players, rewards, IERC20(address(token)), 100 ether);
// Do NOT fund the pot — token balance is 0
​
vm.prank(player);
pot.claimCut();
​
// Player's reward mapping is zeroed but no tokens moved
assertEq(pot.checkCut(player), 0); // zeroed — cannot re-claim
assertEq(token.balanceOf(player), 0); // received nothing
}
}

Recommended Mitigation

Replace bare transfer() / transferFrom() calls with OpenZeppelin's SafeERC20 library, which reverts on false return values and handles non-standard tokens (no-return, false-return).

// Pot.sol
+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract Pot is Ownable(msg.sender) {
+ using SafeERC20 for IERC20;
// ...
​
function closePot() external onlyOwner {
// ...
- i_token.transfer(msg.sender, managerCut);
+ i_token.safeTransfer(msg.sender, managerCut);
// ...
}
​
function _transferReward(address player, uint256 reward) internal {
- i_token.transfer(player, reward);
+ i_token.safeTransfer(player, reward);
}
}
Updates

Lead Judging Commences

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