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

Division by zero in closePot when no players claim locks all funds permanently

wanzablock

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • The normal behavior should be: if no one claims, the remaining funds should either be sent to the manager or held for manual recovery

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

  • The closePot() function divides by claimants.length without checking if any players actually claimed their rewards.

// Root cause in the codebase with @> marks to highlight the relevant section
// Pot.sol - Lines 49-62
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
// @audit No check if claimants.length > 0
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length; // PANIC if length == 0
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
// @audit If no claimants, remaining funds are locked forever
}
}

Risk

Likelihood:

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

  • Medium - This occurs when:

    • Contest gets no participation (niche/unpopular contests)

    • All players miss the 90-day claim window (forgotten contest)

  • Reason 2

Impact:

  • Impact 1

  • Critical Impact:

    1. Total fund lockup: 100% of remaining funds permanently inaccessible

    2. No recovery mechanism: Contract has no way to retrieve funds after revert

  • Impact 2

Proof of Concept

// Test: Zero claimants causes permanent lockup
function testDivisionByZeroLockup() public {
// Create pot with 10 players, 10,000 tokens total
address[] memory players = new address[](10);
uint256[] memory rewards = new uint256[](10);
for (uint i = 0; i < 10; i++) {
players[i] = address(uint160(i + 1));
rewards[i] = 1000;
}
pot = new Pot(players, rewards, token, 10000);
token.transfer(address(pot), 10000);
// NO PLAYERS CLAIM (claimants.length == 0)
// Fast forward 90 days
vm.warp(block.timestamp + 91 days);
// Verify pot has full balance
assertEq(token.balanceOf(address(pot)), 10000);
assertEq(pot.getRemainingRewards(), 10000);
// Manager tries to close pot
vm.expectRevert(stdError.divisionError); // Panic(0x12)
pot.closePot();
// Funds are now PERMANENTLY LOCKED
assertEq(token.balanceOf(address(pot)), 10000); // Still stuck
// Contract is frozen - can never be closed
// No recovery function exists
// 10,000 tokens lost forever
}

Recommended Mitigation

- remove this code
+ add this code// Pot.sol
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = (remainingRewards * managerCutPercent) / 100;
- i_token.transfer(msg.sender, managerCut);
+ // Check if anyone claimed
+ if (claimants.length == 0) {
+ // No claimants - send all remaining rewards to manager
+ i_token.safeTransfer(msg.sender, remainingRewards);
+ emit PotClosedWithNoClaimants(remainingRewards);
+ } else {
+ // Normal distribution
+ i_token.safeTransfer(msg.sender, managerCut);
+
uint256 remainingAfterCut = remainingRewards - managerCut;
- uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
+ uint256 claimantCut = remainingAfterCut / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
+ i_token.safeTransfer(claimants[i], claimantCut);
}
+ // Handle dust
+ uint256 distributed = (claimantCut * claimants.length) + managerCut;
+ uint256 dust = remainingRewards - distributed;
+ if (dust > 0) {
+ i_token.safeTransfer(msg.sender, dust);
+ }
+
+ emit PotClosedWithClaimants(claimants.length, remainingAfterCut);
+ }
}
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 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!