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

closePot() takes a manager cut even when the contest fails, rewarding the manager at the expense of contributors with no recourse

squashingbugs007

Root + Impact

Description

  • Normal behavior: The manager cut is intended as compensation for successfully running a contest. A 10% fee taken from leftover rewards after legitimate claimants have been paid. The protocol documentation describes this as the manager taking "a cut of the remaining pool" after the claim period, implying a successfully concluded contest.

  • The issue: closePot() has exactly one guard, the 90 day time check. It has no check on whether the contest was ever funded, whether anyone claimed, or whether the contest succeeded in any meaningful way. The manager receives 10% of remainingRewards unconditionally.

  • This means: if a pot is created and funded, but zero players ever claim, perhaps because the pot was never announced, players were never notified, or the contest was abandoned, the manager still collects 10% of the entire pool when they call closePot(). The remaining 90% is then split among zero claimants (loop does not execute), and is permanently locked due to the divisor bug in Submission 1. Players who were enrolled receive nothing and have no refund path and claimCut() is only callable before the deadline.

// Pot.sol lines 49-61
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
// @> Manager always takes 10% — no check on whether goal was met
// @> No check on whether anyone claimed
// @> No check on whether pot was ever funded
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

Risk

Likelihood:

  • Any contest that fails to attract participation triggers this: abandoned contests, misconfigured contests, contests where players were not properly notified.

  • The manager controls when to call closePot() and has financial incentive to do so even on failed contests.

  • There is no minimum participation requirement anywhere in the protocol.

Impact:

  • Manager profits from failed contests at the expense of enrolled players.

  • Enrolled players with legitimate reward allocations lose their funds with zero recourse, claimCut() reverts after the deadline, and there is no refund function.

  • Combines with the wrong divisor bug to ensure the remaining 90% after manager cut is also locked, compounding total losses.

Proof of Concept

The following test demonstrates a contest where no players claim. The manager still receives their 10% cut when closing, while all enrolled players receive nothing and cannot recover their allocated rewards.

function test_managerProfitsFromFailedContest() public {
// Create pot with 5 players, 500e18 total rewards
address[] memory players = new address[](5);
uint256[] memory rewards = new uint256[](5);
for (uint256 i = 0; i < 5; i++) {
players[i] = address(uint160(i + 1));
rewards[i] = 100e18;
}
​
Pot pot = new Pot(players, rewards, token, 500e18);
token.transfer(address(pot), 500e18);
​
// No players claim — contest fails
// Advance past 90 day deadline
vm.warp(block.timestamp + 91 days);
​
uint256 managerBefore = token.balanceOf(address(this));
pot.closePot();
uint256 managerAfter = token.balanceOf(address(this));
​
// Manager received 10% = 50e18 despite zero successful claims
assertEq(managerAfter - managerBefore, 50e18);
​
// Players cannot recover their allocated rewards
// claimCut() would revert — deadline has passed
// No refund function exists
vm.prank(players[0]);
vm.expectRevert(); // no way to recover funds
pot.claimCut();
}

Recommended Mitigation

The manager cut should only be taken when at least one player has successfully claimed. If nobody claimed, the remaining funds should be refundable to enrolled players or returned via a separate mechanism. At minimum, a guard on claimants.length should gate the manager cut (the code submitted). A more complete fix would add a refund() function allowing enrolled players to withdraw their allocated amounts if the contest closes with zero participation.

if (remainingRewards > 0) {
+ // Only take manager cut if contest had participation
+ if (claimants.length == 0) {
+ // No one claimed — allow enrolled players to reclaim their allocation
+ // or revert to signal contest must be handled differently
+ revert Pot__NoClaimants();
+ }
+
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
Updates

Lead Judging Commences

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

[M-01] Incorrect Handling of Zero Claimants in `closePot()` Function

## Description In the \`closePot\` function, if the number of claimants is zero, the remaining rewards intended for distribution among claimants may not be properly reclaimed by the Contest Manager. The \`claimantCut\` is calculated using the length of the \`i_players\` array instead of the \`claimants\` array, which could lead to incorrect distribution. Additionally, the function does not have a mechanism to handle the scenario where there are zero claimants, resulting in the potential loss of rewards. ## Vulnerability Details Specifically, when there are no claimants: - The manager's cut is calculated but only a portion or none of the remaining rewards is transferred back to the Contest Manager. - The rewards intended for claimants (\`claimantCut\`) are not distributed because the loop iterating over \`claimants\` does not execute, but there's also no fallback to reclaim these rewards. ## Proof of Concept Add this test in the TestMyCut.t.sol: ```markdown function testClosePotWithZeroClaimants() public mintAndApproveTokens { vm.startPrank(user); // Step 1: Create a new contest contest = ContestManager(conMan).createContest(players, rewards, IERC20(weth), totalRewards); // Step 2: Fund the pot ContestManager(conMan).fundContest(0); // Step 3: Move forward in time by 90 days so the pot can be closed vm.warp(block.timestamp + 90 days); // Step 4: Close the pot with 0 claimants uint256 managerBalanceBefore = weth.balanceOf(user); ContestManager(conMan).closeContest(contest); uint256 managerBalanceAfter = weth.balanceOf(user); vm.stopPrank(); // Step 5: Assert that the Contest Manager received all the remaining rewards // Since there are no claimants, the manager should receive all remaining rewards assertEq(managerBalanceAfter, managerBalanceBefore + totalRewards, "Manager did not reclaim all rewards after closing pot with zero claimants."); ``` In the test `testClosePotWithZeroClaimants`, after closing a pot with zero claimants, the Contest Manager is unable to reclaim all the remaining rewards: ```markdown ├─ [9811] ContestManager::closeContest(Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0]) │ ├─ [8956] Pot::closePot() │ │ ├─ [5288] 0x5929B14F2984bBE5309c2eC9E7819060C31c970f::transfer(ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353], 0) │ │ │ ├─ emit Transfer(from: Pot: [0x43e82d2718cA9eEF545A591dfbfD2035CD3eF9c0], to: ContestManager: [0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353], value: 0) ``` ## Impact - This bug can lead to incomplete recovery of rewards by the Contest Manager. If no participants claim their rewards, a significant portion of the remaining tokens could remain locked in the contract indefinitely, leading to financial loss and inefficient fund management. - And All the reward is lost except from the little 10 % the manager gets because there was no mechanism to claim the remainingReward ## Recommendations - Adjust Calculation Logic: Modify the \`claimantCut\` calculation to divide by \`claimants.length\` instead of \`i_players.length\`. This ensures that only the claimants are considered when distributing the remaining rewards. - Handle Zero Claimants: Implement a check to determine if there are zero claimants. If true, all remaining rewards should be transferred back to the Contest Manager to ensure no tokens are left stranded in the contract. Example ```markdown if (claimants.length == 0) { i_token.transfer(msg.sender, remainingRewards); } else { for (uint256 i = 0; i < claimants.length; i++) { \_transferReward(claimants[i], claimantCut); } } ``` This approach ensures that in the event of zero claimants, all remaining rewards are securely returned to the Contest Manager.

Support

FAQs

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

Give us feedback!