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

claimCut() can be invoked before the contest is funded because the token transfer in Pot constructor is commented out

virgilbb

Root + Impact

Description

Pot constructor line 30 has the funding transfer commented out:

// Pot.sol constructor line 30
// i_token.transfer(address(this), i_totalRewards); // @> COMMENTED OUT

Funding is performed separately via ContestManager.fundContest(). There is no isFunded flag, no balance check, and no guard in claimCut() to verify the Pot holds tokens before attempting to distribute them. A player can call claimCut() on an unfunded Pot immediately after createContest() — the call reverts with an opaque ERC20InsufficientBalance error from the token contract, giving no indication that the Pot hasn't been funded yet.

More critically: the state changes in claimCut() execute before _transferReward:

function claimCut() public {
playersToRewards[player] = 0; // zeroed before transfer
remainingRewards -= reward; // decremented before transfer
claimants.push(player); // added before transfer
_transferReward(player, reward); // reverts if unfunded — tx rolls back
}

While the EVM revert undoes state changes in a failed call, there is no protocol-level protection. If the Pot is partially funded (e.g. dust amount), a player's claim could partially succeed or behave unexpectedly.

Risk

Likelihood: Every contest goes through a creation → funding gap. Any player aware of the Pot address can attempt claimCut() during that window.

Impact: Players receive confusing errors with no clear reason. If the Pot is ever partially funded by mistake, reward accounting becomes inconsistent.

Proof of Concept

The test creates a contest but intentionally skips fundContest. The Pot holds zero tokens. A player immediately calls claimCut(), which proceeds through all state changes and then hits _transferReward, which delegates to the ERC-20 transfer. Because the Pot has no balance, the token contract reverts with ERC20InsufficientBalance — not a Pot-level error. The player has no way to distinguish "not yet funded" from "already closed" or "address not a player".

function testClaimBeforeFundingReverts() public {
vm.startPrank(owner);
address pot = contestManager.createContest(players, rewards, IERC20(weth), totalRewards);
// fundContest intentionally NOT called
vm.stopPrank();
​
// Player immediately tries to claim — gets opaque ERC20 error, not Pot__NotFunded
vm.prank(players[0]);
vm.expectRevert(); // ERC20InsufficientBalance, not a Pot error
Pot(pot).claimCut();
}

Recommended Mitigation

Add an isFunded flag and guard claimCut:

+ bool private s_isFunded;
+ error Pot__NotFunded();
​
function claimCut() public {
+ if (!s_isFunded) revert Pot__NotFunded();
...
}

Set s_isFunded = true inside a receiveFunding() function called by ContestManager after the token transfer. Alternatively, uncomment the constructor transfer and fund atomically at creation.

Updates

Lead Judging Commences

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