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

Pot is funded after deployment , could drain an empty pot

mandech

[H-1] Race condition between createContest() and fundContest() allows permanent erasure of player reward claims

Description

Pot contracts are deployed with reward mappings fully initialised but hold zero tokens until a separate fundContest() call transfers funds. Any eligible player who calls claimCut() during this window will have their reward mapping zeroed out and remainingRewards decremented — with no tokens ever received — because the underlying ERC20 transfer() return value is never checked. The player's claim is permanently destroyed; they cannot claim again after funding arrives.

Vulnerability Details

Root cause: Two-step deploy-then-fund design with no balance guard in claimCut().

The Pot constructor originally contained a commented-out funding line that would have resolved this atomically:

// Pot.sol — constructor
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
​
// i_token.transfer(address(this), i_totalRewards); ← COMMENTED OUT — root cause of window
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}

Because claimCut() performs no balance check and the ERC20 transfer() is unchecked, a call against an empty pot silently succeeds:

// Pot.sol::claimCut()
function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0; // ← mapping wiped
remainingRewards -= reward; // ← accounting decremented
claimants.push(player);
_transferReward(player, reward); // ← 0-token transfer: silent non-revert
}
​
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward); // ← return value ignored
}

Attack / accidental trigger scenario:

  1. Admin calls createContest() → Pot is deployed, token balance = 0, reward mappings populated.

  2. A player (or front-running bot) calls claimCut() before fundContest() lands.

  3. playersToRewards[player] is set to 0 and remainingRewards is reduced.

  4. _transferReward() calls i_token.transfer(player, reward) — contract has no tokens, many ERC20s silently return false here rather than reverting.

  5. Admin calls fundContest() — tokens now arrive, but the player's entry is already gone.

  6. The player calls claimCut() again → reward = 0 → Pot__RewardNotFound revert. Reward is permanently lost.

This can occur accidentally (player is simply fast) or be exploited by a MEV bot scanning the mempool for createContest transactions.

Impact

Severity: HIGH

  • Likelihood: Medium — any player can trigger this accidentally; MEV bots make it near-certain in adversarial conditions.

  • Impact: Critical — a player's entire reward entitlement is permanently destroyed with no recovery path. The tokens are later deposited into the pot by the admin but become unclaimable dust trapped until closePot(), where they are distributed to other claimants.

  • Affects every Pot created through ContestManager, across all contests.

Proof of concept:

function test_RewardErasedBeforeFunding() public {
address[] memory players = new address[](1);
players[0] = alice;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 100e18;
​
// Step 1: Create contest — pot is empty
vm.prank(owner);
address potAddr = manager.createContest(players, rewards, token, 100e18);
Pot pot = Pot(potAddr);
​
// Step 2: Alice claims BEFORE funding
vm.prank(alice);
pot.claimCut(); // succeeds silently — alice receives 0 tokens
​
// Step 3: Admin funds the pot
vm.prank(owner);
manager.fundContest(0); // 100e18 tokens now sit in pot
​
// Step 4: Alice tries to claim her real reward — reverts
vm.prank(alice);
vm.expectRevert(Pot.Pot__RewardNotFound.selector);
pot.claimCut(); // alice's slot was already wiped
​
// Alice received nothing. 100e18 stuck in pot.
assertEq(token.balanceOf(alice), 0);
assertEq(token.balanceOf(potAddr), 100e18);
}

Recommended Mitigation

Option A (Preferred) — Atomic funding at deployment. Pull the tokens inside createContest() in the same transaction:

function createContest(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) public onlyOwner returns (address) {
// Check allowance before deploy
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
Pot pot = new Pot(players, rewards, token, totalRewards);
// Fund atomically in the same tx
token.transferFrom(msg.sender, address(pot), totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}

Remove fundContest() entirely, or retain it only for top-up scenarios with a funded guard.

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!