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

Logic flaw in `Pot::closePot()` causes invalid reward distribution

Turetos

Summary

The Pot::closePot() function is in charge to distribute the rewards to those who claimed in time.
The problem is that the cut for each claimant is calculated dividing the remaing, after the manager cut, using the amount of players in the
pot, instead of the claimants.

The documentation states the remainder is distributed equally to those who claimed in time!. This is the claimants mapping.

There is a second small issue. The remainingRewards, its not updated in this function. This value is used in Pot::getRemainingRewards(),
which in turn is called by ContestManager::getContestRemainingRewards(). Any App using this function will get a wrong value.

Vulnerability Details

  • claimantCut is calculated using the total number of players instead of claimants.

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);
}
}
}
}

Impact

The reward distribution is incorrect, potentially locking funds in the contract permanently, as there is no function to recover stuck tokens.
Below is a test case with two players, where only one claims. A pot holds 1000 tokens, with 500 rewards allocated to each player.

  • We have a pot with 1000 tokens.

  • One player claims 500 tokens, leaving 500 remaining in the pot (1000 - 500).

  • The pot is closed, and the manager claims a 10% cut of the remaining 500 tokens, which is 50 tokens.

  • The pot contains 450.

  • The claimant's cut is calculated as the remainder of the pot divided by the total players: 450 / 2 = 225.

  • 250 tokens are given to the player who claimed on time.

  • 250 are stuck in the contract

function testRemainderNotEquallyDistributed() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
console.log("TOTAL POT ::", weth.balanceOf(contest));
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
console.log("POT AFTER player1 CLAIM ::", weth.balanceOf(contest));
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
console.log("POT AFTER CLOSE CONTEST ::", weth.balanceOf(contest));
}
​

Output

Logs:
TOTAL POT :: 1000
POT AFTER player1 CLAIM :: 500
POT AFTER CLOSE CONTEST :: 225

Tools Used

Manual review and Foundry

Recommendations

  • Calculate claimantCut using claimants.length instead of i_players.length.

  • Update remainingRewards to reflect no remaining rewards.

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
++ remainingRewards = 0;
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
​
-- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
++ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect distribution in closePot()

Support

FAQs

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