MyCut

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

claimCut Has No Funded Check — Callable Before fundContest Transfers Tokens

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • Explain the specific issue or problem in one or more sentences

/# [M-02] claimCut Has No Funded Check — Callable Before fundContest Transfers Tokens
### Summary
`Pot.sol` does not check whether the pot has been funded by `fundContest` before allowing claims. While the transaction currently reverts atomically due to an ERC20 insufficient balance error inside `_transferReward`, there is no explicit check or guard preventing the call, meaning the protocol relies entirely on call ordering with no strict enforcement.
### Vulnerability Detail
Lines 37-47 in `Pot.sol` allow users to invoke `claimCut()`. If a user invokes this function before `fundContest` has successfully transferred tokens into the `Pot` contract, the internal state transitionssuch as zeroing out the user's reward mapping, subtracting from `remainingRewards`, and pushing to the `claimants` arraywould attempt to execute. Ultimately, the transaction reverts because the contract's token balance is zero. However, relying purely on ERC20 transfer failures for flow control creates architectural fragility. If the protocol is later updated, or if gas token dynamics change, this lack of enforcement can lead to unexpected edge cases.
### Impact
Fragile protocol flow. Callers will waste gas executing transactions that are guaranteed to fail due to missing lifecycle state enforcement.
### Affected Code
* `Pot.sol`
### Proof of Concept
Add the following test case to your test suite to demonstrate that the lifecycle check is missing:
```solidity
function test_M02_ClaimCutBeforeFunding() public {
// Create contest but DO NOT fund it
address[] memory players = new address[](2);
players[0] = player1;
players[1] = player2;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 500e18;
rewards[1] = 500e18;
vm.prank(owner);
conMan.createContest(players, rewards);
address contest = conMan.contests(0);
// fundContest NOT called
// Player tries to claim — reverts because Pot has 0 tokens
vm.prank(player1);
vm.expectRevert(); // ERC20 transfer fails — insufficient balance
Pot(contest).claimCut();
// Pot still has 0 tokens, player's reward mapping unchanged
assertEq(Pot(contest).checkCut(player1), 500e18);
assertEq(weth.balanceOf(contest), 0);
}
```
### Tools Used
* Manual Review
### Recommendations
Introduce a explicit state validation check by utilizing a initialization/funded boolean flag inside the contract state:
```solidity
// In Pot.sol:
bool public funded;
function fund() external onlyOwner {
require(!funded, "Already funded");
funded = true;
i_token.transferFrom(msg.sender, address(this), totalRewards);
}
function claimCut() public {
require(funded, "Pot not yet funded"); // Explicit lifecycle check
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);
}
```
/ Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Impact 1

  • Impact 2

Proof of Concept

Recommended Mitigation

- remove this code
+ add this code
Updates

Lead Judging Commences

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