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

Function ignores return value resulting in rewards loss

joshuapaul1

Root + Impact

Description

  • Players can claim their cut using ClaimCut() function , which calls the another function _transferReward()

  • The _tranferReward() function ignores the boolean return value of the i_token.transfer() function call. It state-updates a player's reward balance to zero before verifying if the token transfer actually succeeded.

// @> line 44 - line 54
​
function claimCut() public {
.....
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
//here
_transferReward(player, reward);
}
​
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward);
/**
token transfer ignores return value resulting in tranfer failres
eg l update thinking the player received their tokens. The player gets 0 actual tokens, but your contract marks the reward as paid.
*/
}
​
​

Risk

Likelihood:

  • High. since it's the main function through which players/ contenstants are able to receive their rewards , hence this vulnerability is likely to occur


Impact:

  • High: It results in a permanent loss of funds/rewards for legitimate users without a path for recovery.


Proof of Concept

This test sets up the contract without funding it. It then forces a player to claim, verifying the broken state modification.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test, console} from "forge-std/Test.sol";
import {Pot} from "../src/Pot.sol"; // Adjust path to your contract
import {MockBadERC20} from "./MockBadERC20.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
contract PotPoCTest is Test {
Pot public pot;
MockBadERC20 public badToken;
​
address public player1 = address(0x1111);
address public player2 = address(0x2222);
​
function setUp() public {
badToken = new MockBadERC20();
​
address[] memory players = new address[](2);
players[0] = player1;
players[1] = player2;
​
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100 ether;
rewards[1] = 200 ether;
​
uint256 totalRewards = 300 ether;
​
// Deploying the Pot contract WITHOUT funding it
// Simulates an underfunded pot or failed initial funding
pot = new Pot(players, rewards, IERC20(address(badToken)), totalRewards);
}
​
function test_poc_unchecked_transfer_wipes_rewards() public {
// 1. Confirm the Pot contract has a token balance of 0
assertEq(badToken.balanceOf(address(pot)), 0);
​
// 2. Confirm player1 has a pending reward of 100 ether inside contract storage
uint256 rewardBefore = pot.checkCut(player1);
assertEq(rewardBefore, 100 ether);
console.log("Player 1 starting reward balance in Pot:", rewardBefore);
​
// 3. Player1 attempts to claim their reward
vm.prank(player1);
pot.claimCut();
// Note: The tx succeeds and does NOT revert, despite the contract having 0 tokens!
​
// 4. Check actual wallet token balance of player1
uint256 actualTokenBalance = badToken.balanceOf(player1);
assertEq(actualTokenBalance, 0);
console.log("Player 1 actual wallet token balance after claim:", actualTokenBalance);
​
// 5. Check the updated reward tracking inside Pot contract
uint256 rewardAfter = pot.checkCut(player1);
assertEq(rewardAfter, 0);
console.log("Player 1 reward tracking inside Pot after claim:", rewardAfter);
​
// 6. Prove player1 is permanently locked out from ever trying to claim again
vm.expectRevert(Pot.Pot__RewardNotFound.selector);
vm.prank(player1);
pot.claimCut();
console.log("PoC Success: Player lost entitlement with 0 tokens received.");
}
}

Run this command in your Foundry project terminal terminal to see the logs:

forge test --match-test test_poc_unchecked_transfer_wipes_rewards -vvvv

Recommended Mitigation

To fix this vulnerability, use OpenZeppelin's SafeERC20 library. This library wraps standard ERC-20 operations and forces a hard transaction revert if a transfer returns false.

Step-by-Step Fixes:

  1. Import SafeERC20 from the OpenZeppelin library.

  2. Apply using SafeERC20 for IERC20; to the contract.

  3. Replace .transfer() with .safeTransfer().

  4. Implement a Checks-Effects-Interactions pattern (though safeTransfer protects you, keeping the state mutation before external calls is best practice).

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!