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

A Portion of Funds is Lost Because the ClaimantCut Calculations Are Incorrect

4rdiii

[H-02] A Portion of Funds is Lost Because the ClaimantCut Calculations Are Incorrect

Summary

The calculations for distributing rewards among claimants in the Pot::closePot function are incorrect. Instead of dividing the remaining rewards (after subtracting the manager's cut) by the number of claimants, the function divides by i_players.length, which causes some funds to be left unclaimed in the contract.

Vulnerability Details

In the Pot::closePot function, the managerCut is correctly calculated as one-tenth of the remaining rewards. However, the claimantCut calculation incorrectly uses i_players.length instead of claimants.length. This discrepancy results in some rewards being left in the contract, as the actual number of claimants is not correctly accounted for.

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

A portion of the rewards will remain unclaimed in the contract, leading to a permanent loss of funds.

Proof of Concept

Add this to the following test suite to reproduce the issue:

function test_my_wrongClaimantCutCalculations()
public
mintAndApproveTokens
{
address[] memory players_ = new address[](15);
uint256[] memory rewards_ = new uint256[](15);
​
for (uint256 i; i < 15; i++) {
players_[i] = address(uint160(i + 10));
rewards_[i] = 1 ether;
}
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
players_,
rewards_,
IERC20(ERC20Mock(weth)),
15 ether
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
for (uint256 i; i < 11; i++) {
vm.prank(players_[i]);
Pot(contest).claimCut();
}
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
assertEq(weth.balanceOf(conMan), (1000e18 - 15e18) + 0.4 ether); // manager cut = 0.4 ether
assertEq(weth.balanceOf(players_[1]), 1.24 ether); // claiment cut which is incorrectly calculated
assertEq(weth.balanceOf(contest), 0.96 ether); // stuck in the pot!
​
// 15 - 11 = 4 ether left
// 4 / 10 = 0.4
// 11 have claimed, 4-0.4 = 3.6 is left, 3.6/11 = 0.3272727... + 1 ether from before = 1.3272727... ether
// but we got 3.6/15 = 0.24, + 1 ether from before = 1.24 ether
}

Tools Used

Manual Review

Recommendations

Change the division in the claimantCut calculation from i_players.length to claimants.length to ensure that the rewards are distributed correctly among the actual number 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;
+ claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
Updates

Lead Judging Commences

equious Lead Judge 12 months 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.