MyCut

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

Unchecked ERC20 transfer/transferFrom return values — a false-returning token silently loses a claimant's reward

Description

Every token movement in the protocol uses the raw return value of IERC20.transfer / transferFrom without checking it:

// Pot.sol
i_token.transfer(msg.sender, managerCut); // closePot
i_token.transfer(player, reward); // _transferReward (used by claimCut and closePot)
// ContestManager.sol
token.transferFrom(msg.sender, address(pot), totalRewards); // fundContest

An ERC20 that signals failure by returning false instead of reverting will make these calls appear to succeed while moving no tokens. The consequences are silent and state-corrupting:

  • In claimCut, the player's allocation is zeroed (playersToRewards[player] = 0) and remainingRewards is decremented before _transferReward. If the transfer returns false, the player's reward is permanently lost — their allocation is gone but they received nothing.

  • In closePot, a failed manager-cut or claimant transfer is ignored, so the accounting (remainingRewards, the manager's expectation) silently diverges from the tokens actually moved.

  • In fundContest, a transferFrom that returns false leaves the pot marked as funded (contestToTotalRewards) while holding no tokens; later claimCut/closePot transfers then revert on insufficient balance.

Risk

Impact: Medium. Silent loss of a claimant's reward and corrupted pot accounting on any token that returns false rather than reverting.

Likelihood: Low–Medium. The contest scope states "Standard ERC20 Tokens Only," which reduces exposure — but a number of widely-used, nominally-standard tokens do not revert on failed transfers, and the code uses the same unchecked pattern in the critical claimCut path where state is updated before the transfer.

Proof of Concept

// A standard-shaped ERC20 whose transfer returns false instead of reverting:
contract FalseReturningToken is ERC20Like {
function transfer(address, uint256) external returns (bool) { return false; } // no revert
}
function test_claimCutLosesRewardOnFalseReturn() public {
// pot funded with FalseReturningToken; player has an allocation
uint256 before = token.balanceOf(player);
vm.prank(player);
pot.claimCut(); // does NOT revert; playersToRewards[player] set to 0
assertEq(token.balanceOf(player), before); // player received nothing
assertEq(pot.checkCut(player), 0); // ...but their allocation is now zero -> reward lost
}

Expected: a failed transfer reverts the whole claim so the allocation is preserved. Actual: the claim succeeds, the allocation is zeroed, and the reward is gone.

Recommended Mitigation

Use OpenZeppelin SafeERC20 for every token movement, which reverts when the token returns false (or returns no data unexpectedly):

using SafeERC20 for IERC20;
i_token.safeTransfer(msg.sender, managerCut);
i_token.safeTransfer(player, reward);
token.safeTransferFrom(msg.sender, address(pot), totalRewards);

safeTransfer/safeTransferFrom revert on a false or malformed return, so a failed transfer rolls back the state changes (the allocation stays intact and can be re-claimed) instead of silently losing funds.

Updates

Lead Judging Commences

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