MyCut

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

closePot Divides Remainder by Total Players Instead of Claimants — Majority of Funds Permanently Locked

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

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

# [H-#] `closePot` Divides Remainder by Total Players Instead of Claimants — Majority of Funds Permanently Locked
## Summary
The `closePot` function in `Pot.sol` calculates the remainder payout distribution by dividing the leftover rewards by the total initial player count (`i_players.length`) rather than the active number of validated claimants (`claimants.length`). This mathematical mismatch results in each claimant receiving only a small fraction of their intended share, while the unallocated remainder tokens become permanently locked in the contract with no recovery path.
## Vulnerability Details
In `Pot.sol`, the logic responsible for calculating each claimant's remaining bonus allocation uses the following division formula:
```solidity
// @audit-issue i_players.length includes non-claimants, producing an artificially small distribution slice
claimantCut = (remainingRewards - managerCut) / i_players.length;
```
According to the protocol specification, any residual reward funds remaining after the initial distribution period must be split evenly among the users who actually came forward to claim their rewards (the active claimants).
By utilizing the static `i_players.length` value as the mathematical divisor instead of the dynamic `claimants.length` size, the contract calculates a payout fraction that assumes every single initialized player has filed a valid claim. When fewer players file claims than the total pool size (`claimants.length < i_players.length`), the calculated allocation per user drops significantly. The undistributed difference remains completely unassigned in the contract's storage structures, trapping those tokens permanently.
## Impact
**High.** Capital efficiency is significantly compromised, resulting in direct token loss for legitimate users. Any context where the full list of players does not submit active claims will cause a large percentage of the residual rewards pool to freeze inside the contract permanently.
## Proof of Concept
Add the following test case to your suite to demonstrate the math divergence and token lockup:
```solidity
function test_H02_WrongDivisorLocksFunds() public {
// 1. Setup a contest containing 5 total initialized players (Total Pool = 1,000 tokens)
// 2. Only 2 out of the 5 players actively invoke claimCut(), leaving a remainder balance of 600 tokens
// 3. Process the contest closure routine past the lock period
// Total Remainder to distribute after 10% manager fee (60 tokens) = 540 tokens
// Mathematical calculation executed by contract: 540 / 5 (total players) = 108 tokens per claimant
// Mathematical expectation by specification: 540 / 2 (actual claimants) = 270 tokens per claimant
// 4. Verify the capital deficit trapped forever inside the contract balance sheet
// 540 - (108 * 2) = 324 tokens remaining unallocated
uint256 stuckTokens = token.balanceOf(address(pot)) - managerCut;
assertEq(stuckTokens, 324e18);
console.log("Tokens permanently frozen due to incorrect divisor:", stuckTokens);
}
```
## Tools Used
* Manual Code Review
## Recommendations
Update the dividend scaling calculation to compute individual distribution slices against the true length of the active `claimants` collection array instead of the static initial deployment player count:
```solidity
function closePot() external {
// Ensure active claimants exist to avoid a potential division-by-zero panic
uint256 totalClaimants = claimants.length;
if (totalClaimants == 0) {
// Handle edge case where no one claimed anything
// ...
return;
}
// @audit-issue Divide by the actual claimants length to ensure accurate distribution of the entire remainder
claimantCut = (remainingRewards - managerCut) / totalClaimants;
// ... existing logic to process transfers
}
```
*Note: Since integer division in Solidity truncates downwards, consider adding a dust collection accumulator or routing any tiny fractional precision remainders (dust) straight to the manager fee recipient address to fully clear out the token balance.*
// 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:

[H-02] Incorrect logic in `Pot::closePot` leads to unfair distribution to `claimants`, potentially locking the funds with no way to take that out

## Description in `closePot` function while calclulating the shares for claimaint cut, `i_players.length` is used, instead of `claimants.length`, causing low amount being distributed to claimants. ## Vulnerability Details [2024-08-MyCut/src/Pot.sol at main · Cyfrin/2024-08-MyCut (github.com)](https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/Pot.sol#L57) `Pot::closePot` function is meant to be called once contest passed 90 days, it sends the owner cut to owner and rest is splitted among the users who claimed b/w 90 days period. However, current implementation is wrong.&#x20; It uses total users (i_players.length) instead of the users (claimants.length) who claimed during the duration. This creates an unfair distribution to the participants and some of the funds could be locked in the contract. In worst case scenerio, it could be 90% if nobody has claimed from the protocol during the 90 days duration. ## POC In existing test suite, add following test: ```solidity function testUnfairDistributionInClosePot() public mintAndApproveTokens { // Setup address[] memory testPlayers = new address[](3); testPlayers[0] = makeAddr("player1"); testPlayers[1] = makeAddr("player2"); testPlayers[2] = makeAddr("player3"); uint256[] memory testRewards = new uint256[](3); testRewards[0] = 400; testRewards[1] = 300; testRewards[2] = 300; uint256 testTotalRewards = 1000; // Create and fund the contest vm.startPrank(user); address testContest = ContestManager(conMan).createContest( testPlayers, testRewards, IERC20(ERC20Mock(weth)), testTotalRewards ); ContestManager(conMan).fundContest(0); vm.stopPrank(); // Only player1 claims their reward vm.prank(testPlayers[0]); Pot(testContest).claimCut(); // Fast forward 91 days vm.warp(block.timestamp + 91 days); // Record balances before closing the pot uint256 player1BalanceBefore = ERC20Mock(weth).balanceOf( testPlayers[0] ); // Close the contest vm.prank(user); ContestManager(conMan).closeContest(testContest); // Check balances after closing the pot uint256 player1BalanceAfter = ERC20Mock(weth).balanceOf(testPlayers[0]); // Calculate expected distributions uint256 remainingRewards = 600; // 300 + 300 unclaimed rewards uint256 ownerCut = remainingRewards / 10; // 10% of remaining rewards uint256 distributionPerPlayer = (remainingRewards - ownerCut) / 1; // as only 1 user claimed uint256 fundStucked = ERC20Mock(weth).balanceOf(address(testContest)); // actual results console.log("expected reward:", distributionPerPlayer); console.log( "actual reward:", player1BalanceAfter - player1BalanceBefore ); console.log("Fund stucked:", fundStucked); } ``` then run `forge test --mt testUnfairDistributionInClosePot -vv` in the terminal and it will show following output: ```js [⠊] Compiling... [⠒] Compiling 1 files with Solc 0.8.20 [⠘] Solc 0.8.20 finished in 1.63s Compiler run successful! Ran 1 test for test/TestMyCut.t.sol:TestMyCut [PASS] testUnfairDistributionInClosePot() (gas: 905951) Logs: User Address: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Contest Manager Address 1: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 Minting tokens to: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Approved tokens to: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 expected reward: 540 actual reward: 180 Fund stucked: 360 Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.58ms (506.33µs CPU time) ``` ## Impact Loss of funds, Unfair distribution b/w users ## Recommendations Fix the functions as shown below: ```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; + uint256 totalClaimants = claimants.length; + if(totalClaimant == 0){ + _transferReward(msg.sender, remainingRewards - managerCut); + } else { + uint256 claimantCut = (remainingRewards - managerCut) / claimants.length; for (uint256 i = 0; i < claimants.length; i++) { _transferReward(claimants[i], claimantCut); } } + } } ```

Support

FAQs

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

Give us feedback!