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

Pot::closePot Divides Remaining Rewards by Total Players Instead of Claimants, Underpaying Claimants and Locking Funds

harmonygemini8

Description

MyCut is a contest rewards distribution protocol. The owner-controlled ContestManager creates Pots; each Pot has a fixed list of players and their reward amounts. Players have 90 days to claim their cut via Pot::claimCut(). After the 90-day window the owner calls closeContest() -> Pot::closePot(), which is supposed to (1) give the manager 10% of the remaining unclaimed rewards, and (2) distribute the other 90% equally among the players who DID claim in time ("the remainder is distributed equally to those who claimed in time" - project README).

The bug is in step 2. Pot::closePot() computes the per-claimant share using i_players.length (the TOTAL number of registered players) instead of claimants.length (the players who actually claimed):

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; // BUG
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

(src/Pot.sol:57-66). The denominator should reflect how many players actually participate in the redistribution (claimants), not the total registered players. When fewer players claim than are registered - the normal case, since some players always miss the window - the per-claimant share is diluted and the leftover has no owner: the loop only iterates over claimants, and no other function in the Pot can pull out the remaining balance, so those tokens are permanently locked.

Concrete numeric walkthrough (also reproduced in the PoC):

  • 2 players [p1, p2], rewards [500, 500], totalRewards 1000, pot funded 1000.

  • p1 claims 500 within the window: remainingRewards = 500, claimants = [p1], pot balance = 500.

  • 91 days later closePot() runs:

    • managerCut = 500 / 10 = 50 -> transferred to ContestManager;

    • claimantCut = (500 - 50) / 2 = 225 -> transferred to p1.

  • Expected correct behavior: p1 should receive (500 - 50) / 1 = 450 and the pot should be empty.

  • Actual result: p1 receives only 225; pot balance = 500 - 50 - 225 = 225 -> 225 tokens permanently locked.

Risk

Impact: Medium - every claimant receives less than entitled, and a portion of the reward pool is permanently locked with no recovery path. With N players and 1 claimant, the claimant gets (remaining x 0.9)/N and (N-1)/N of the remaining pool is locked.
Likelihood: High - the bug triggers on every closePot() in which not all players claimed, which is the expected common scenario (the whole point of "claim in time" is that some players miss the deadline).

Proof of Concept

Foundry test testPocA_ClosePotWrongDivision (passes):

function testPocA_ClosePotWrongDivision() public {
address contest = _createAndFund();
uint256 p1Before = weth.balanceOf(player1);
​
vm.prank(player1);
Pot(contest).claimCut(); // claims 500
​
vm.warp(91 days);
vm.prank(user);
conMan.closeContest(contest);
​
// p1 gets 500 (claim) + 225 (redistributed). Correct math would give 500 + 450.
assertEq(weth.balanceOf(player1) - p1Before, 725);
assertEq(weth.balanceOf(contest), 225); // dust stuck forever
assertEq(weth.balanceOf(address(conMan)), 50); // manager cut 10%
}

Test output: [PASS] testPocA_ClosePotWrongDivision - p1 balance 725; Pot balance 225; ContestManager 50.

Additional observations confirming impact:

  • getRemainingRewards() still returns 500 after closePot() because the state is never reset, so even the contract's own accounting shows the pool is not fully distributed.

  • Since remainingRewards is never updated in closePot(), the owner can call closePot() repeatedly, each time moving another 10% of the unchanged remainingRewards to the ContestManager until the Pot is drained (see related PoC testPocD_ClosePotRepeated).

Recommended Mitigation

Divide by the number of actual claimants and zero out remainingRewards at the end:

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) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
remainingRewards = 0;
}
}

Tools Used

  • Manual code review

  • Foundry (forge test) for the PoC

References

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!