MyCut

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

Division by Zero in closePot() When No Claimants

Root + Impact

Description

  • The `closePot()` function divides the remaining rewards by `claimants.length` without checking if the claimants array is empty. If no players have claimed their rewards within the 90-day period, `claimants.length` will be 0, causing a division by zero error when calculating `claimantCut`. This will cause the transaction to revert, preventing the manager from taking their cut and leaving all unclaimed funds permanently locked in the contract.

    While the function checks `if (remainingRewards > 0)`, it doesn't check if there are any claimants before performing the division.

```solidity
// Root cause in the codebase
53| if (remainingRewards > 0) {
54| uint256 managerCut = remainingRewards / managerCutPercent;
55| i_token.transfer(msg.sender, managerCut);
56|
57| uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
58| for (uint256 i = 0; i < claimants.length; i++) {
59| _transferReward(claimants[i], claimantCut);
60| }
61| }
```
Note: Even with the fix from Issue #1 (using `claimants.length` instead of `i_players.length`), this issue still exists if no one has claimed.

Risk

Likelihood:

  • * This occurs when `closePot()` is called after 90 days and no players have claimed their rewards

    * The issue manifests whenever `claimants.length == 0` and `remainingRewards > 0`

Impact:

  • * Transaction reverts due to division by zero, preventing pot closure

    * Manager cannot take their cut

    * All unclaimed funds remain permanently locked in the contract

    * Protocol fails to fulfill its intended distribution mechanism

Proof of Concept

```solidity
// Scenario: Pot created with 1000 tokens, no players claim within 90 days
// 1. 90 days pass
// 2. Owner calls closePot()
// 3. remainingRewards = 1000 > 0, so enters if block
// 4. managerCut = 1000 / 10 = 100 (succeeds)
// 5. claimantCut = (1000 - 100) / 0 = REVERTS (division by zero)
// 6. Transaction fails, funds remain locked
```

Recommended Mitigation

```diff
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);
- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
- for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
- }
+ if (claimants.length > 0) {
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
+ for (uint256 i = 0; i < claimants.length; i++) {
+ _transferReward(claimants[i], claimantCut);
+ }
+ }
+ // If no claimants, manager gets all remaining rewards
+ // (already transferred managerCut, remaining tokens stay in contract)
}
}
```
Alternatively, give all remaining rewards to manager if no claimants:
```diff
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
- i_token.transfer(msg.sender, managerCut);
+
+ if (claimants.length > 0) {
+ i_token.transfer(msg.sender, managerCut);
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
+ for (uint256 i = 0; i < claimants.length; i++) {
+ _transferReward(claimants[i], claimantCut);
+ }
+ } else {
+ // No claimants, manager gets all remaining rewards
+ i_token.transfer(msg.sender, remainingRewards);
+ }
}
```
Updates

Lead Judging Commences

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