MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Pot__InsufficientFunds declared but never used: missing funding check

sharonkosgei4

Root + Impact

Description

  • Pot__InsufficientFunds is defined but never thrown. The commented-out transfer in the constructor (// i_token.transfer(address(this), i_totalRewards)) suggests a funding guard was intended but removed.

  • As a result, claimCut can be called on an unfunded Pot. Combined with unchecked transfer return values (M1), the player's transfer silently fails while their entitlement is permanently zeroed out.


Risk

Likelihood:

  • Funding a Pot is a separate, manually executed step (fundContest) that is not atomically coupled with Pot creation (createContest).

  • An operator could deploy a Pot without ever calling fundContest or the funding transaction could fail silently if a non-reverting token is used (M-01).

  • In either case, there is no on-chain gate preventing claimCut from executing. Medium likelihood because the two-step create-then-fund flow is easy to get wrong operationally, and the consequence, permanent, silent loss of the player's entire entitlement, is severe.

Proof of Concept

The following test demonstrates that calling claimCut on an unfunded Pot (backed by a non-reverting token) permanently destroys a player's reward entitlement with no tokens received and no informative revert. The player's only fallback — retrying — fails with Pot__RewardNotFound leaving them with no recourse. Run with forge test --match-test testL03 -vvv (requires the ReturnFalseERC20 helper from M1):

function testL3_ClaimOnUnfundedPotZeroesEntitlement() public {
// Use the non-reverting token from M1 to exercise the silent failure path.
// Note: with a standard reverting OZ ERC20, claimCut would still revert —
// but with an ERC20 "insufficient balance" error, not Pot__InsufficientFunds,
// still demonstrating the missing guard.
ReturnFalseERC20 badToken = new ReturnFalseERC20();
// Do NOT mint to the Pot — it stays unfunded.
​
address[] memory players = new address[](1);
players[0] = player1;
uint256[] memory rewards = new uint256[](1);
rewards[0] = TOTAL_REWARDS;
​
vm.startPrank(owner);
badToken.approve(address(conMan), TOTAL_REWARDS);
address contestAddr = conMan.createContest(
players, rewards, IERC20(address(badToken)), TOTAL_REWARDS
);
// fundContest intentionally NOT called — Pot holds 0 tokens
vm.stopPrank();
​
assertEq(badToken.balanceOf(contestAddr), 0, "pot is unfunded");
​
// claimCut does not revert — the balance check is missing.
// The transfer silently returns false (M1), but the entitlement
// mapping is already zeroed before the transfer is even attempted.
vm.prank(player1);
Pot(contestAddr).claimCut(); // no Pot__InsufficientFunds revert
​
// Player1's entitlement is permanently gone
assertEq(badToken.balanceOf(player1), 0, "player1 received nothing");
assertEq(Pot(contestAddr).checkCut(player1), 0, "entitlement permanently zeroed");
​
// Any retry fails with RewardNotFound — the player has no recourse
vm.prank(player1);
vm.expectRevert(Pot.Pot__RewardNotFound.selector);
Pot(contestAddr).claimCut();
​
console.log("--- L-03 Results ---");
console.log("Player1 balance after claim on unfunded pot:", badToken.balanceOf(player1));
console.log("Expected Pot__InsufficientFunds revert — none raised.");
}

Recommended Mitigation

  • Reintroduce the balance check before the reward mapping is cleared, using the already-declared custom error

  • Alternatively, add a funded boolean flag that ContestManager.fundContest sets on the Pot and gate claimCut behind it to prevent any claim before the Pot is funded.

function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
+ if (i_token.balanceOf(address(this)) < reward) {
+ revert Pot__InsufficientFunds();
+ }
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
_transferReward(player, reward);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 4 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[M-02] **[L-1] users can invoke `claimCut` prior to the contest being funded**

````markdown **Description:** It is possible that once the contest has been created, it is not necessarily funded at the same time, these are separate operations, which may result in users attempting to invoke `claimCut`, however there would be no funds and we would most likely get a `ERC20InsufficientBalance` error. Users have most probably assumed that at the time of claiming their cut that the contest is funded. The more insidious issue lies in the fact that the timer of 90 days begins when the Pot contract is constructed not when it's funded, hence if the contract is not funded at the time of creation, users will not be entitled to the whole 90 day duration claim period. **Impact:** Bad UX, as users would be able to attempt claim their cut but this would result in a reversion. **Proof of Concept:** The below test can be added to `TestMyCut.t.sol:TestMyCut` contracts test suite. **Recommended Mitigation:** We must ensure the contest is funded at the time it is created. Otherwise we should state a clearer error message. In the event where we want to give the users a more gracious error message, we could add the following changes which leverages a boolean to track if the Pot has been funded: ```diff contract Pot is Ownable(msg.sender) { /** Existing Code... */ + boolean private s_isFunded; // Ensure this is updated correctly when the contract is funded. function claimCut() public { + if (!s_isFunded) { + revert Pot__InsufficientFunds(); + } address player = msg.sender; uint256 reward = playersToRewards[player]; if (reward <= 0) { revert Pot__RewardNotFound(); } playersToRewards[player] = 0; remainingRewards -= reward; claimants.push(player); _transferReward(player, reward); } } ``` In the scenario where we want to ensure the contest is funded at the time of being created employ the following code. ```diff function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) public onlyOwner returns (address) { // Create a new Pot contract Pot pot = new Pot(players, rewards, token, totalRewards); contests.push(address(pot)); contestToTotalRewards[address(pot)] = totalRewards; + fundContest(contests.length - 1); return address(pot); } - function fundContest(uint256 index) public onlyOwner { + function fundContest(uint256 index) internal onlyOwner { Pot pot = Pot(contests[index]); IERC20 token = pot.getToken(); uint256 totalRewards = contestToTotalRewards[address(pot)]; if (token.balanceOf(msg.sender) < totalRewards) { revert ContestManager__InsufficientFunds(); } token.transferFrom(msg.sender, address(pot), totalRewards); } ``` ````

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!