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

Unchecked ERC20 transfer return values in three locations — silent token loss on failed transfers

squashingbugsforlife

Root + Impact

Description

  • Normal behavior: Token transfers should revert on failure so the protocol never enters an inconsistent state where state is updated but funds are not moved.

  • The issue: Three ERC20 transfer() and transferFrom() calls ignore their return values. For non-reverting ERC20 tokens (e.g. USDT), a failed transfer returns false instead of reverting. The protocol updates state — deducting remainingRewards, pushing to claimants, transferring manager cut — while no tokens actually move. Funds are permanently lost with no error raised.

```solidity
// src/ContestManager.sol#37
// @> return value not checked — fundContest silently fails
token.transferFrom(msg.sender, address(pot), totalRewards);
​
// src/Pot.sol#55
// @> return value not checked — manager cut silently lost
i_token.transfer(msg.sender, managerCut);
​
// src/Pot.sol#65
// @> return value not checked — player reward silently lost
i_token.transfer(player, reward);
```

Risk

Likelihood:

  • Any ERC20 token that returns false instead of reverting on failure triggers silent loss — USDT and similar tokens are widely used.

  • The protocol accepts any standard ERC20 token — no whitelist restricts token behavior.Impact:

  • Player rewards are deducted from remainingRewards and playersToRewards zeroed out — player cannot reclaim even though no tokens were received.

  • Manager cut is transferred from state before token movement — unclaimed rewards are permanently locked in the contract.

Proof of Concept

The following Foundry test demonstrates that claimCut() silently succeeds even when the underlying ERC20 transfer returns false. A mock ERC20 that returns false on transfer() is used. The player's reward is zeroed in state, claimants is updated, but no tokens are received. The player cannot claim again.

```solidity
contract MockFailToken is IERC20 {
mapping(address => uint256) public balanceOf;
function transfer(address, uint256) external pure returns (bool) { return false; }
function transferFrom(address, address to, uint256 amount) external returns (bool) {
balanceOf[to] += amount; return true;
}
function approve(address, uint256) external pure returns (bool) { return true; }
function allowance(address, address) external pure returns (uint256) { return type(uint256).max; }
function totalSupply() external pure returns (uint256) { return 0; }
}
​
function testUncheckedTransferSilentLoss() public {
MockFailToken token = new MockFailToken();
address[] memory players = new address[](1);
uint256[] memory rewards = new uint256[](1);
players[0] = player;
rewards[0] = 100e18;
​
Pot pot = new Pot(players, rewards, IERC20(address(token)), 100e18);
​
// Transfer returns false — state updated, no tokens moved
vm.prank(player);
pot.claimCut();
​
// Player reward zeroed — cannot claim again
assertEq(pot.checkCut(player), 0);
// Player received nothing
assertEq(token.balanceOf(player), 0);
}
```

Recommended Mitigation

Replace all three raw transfer() and transferFrom() calls with OpenZeppelin's SafeERC20 wrappers. safeTransfer() and safeTransferFrom() revert on failure regardless of whether the token returns false or reverts. Import SafeERC20 and apply using SafeERC20 for IERC20 in both contracts.

```diff
// ContestManager.sol
+ import {SafeERC20} from 'lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol';
contract ContestManager is Ownable {
+ using SafeERC20 for IERC20;
// ...
- token.transferFrom(msg.sender, address(pot), totalRewards);
+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);
​
// Pot.sol
+ import {SafeERC20} from 'lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol';
contract Pot is Ownable(msg.sender) {
+ using SafeERC20 for IERC20;
// ...
- i_token.transfer(msg.sender, managerCut);
+ i_token.safeTransfer(msg.sender, managerCut);
// ...
- i_token.transfer(player, reward);
+ i_token.safeTransfer(player, reward);
}
```
Updates

Lead Judging Commences

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