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

Incorrect Distribution of Remaining Rewards (Division by Total Players Instead of Claimants)

princekay

[S-1] Incorrect Distribution of Remaining Rewards (Division by Total Players Instead of Claimants)

Description:

The closePot function incorrectly calculates and distributes the remaining rewards when the pot is closed. Specifically, the calculation of claimantCut is performed by dividing the remaining rewards (after deducting the manager's cut) by the total number of players (i_players.length) instead of the number of actual claimants. This results in an incorrect and insufficient distribution of rewards to claimants.

https://github.com/Cyfrin/2024-08-MyCut/blob/946231db0fe717039429a11706717be568d03b54/src/Pot.sol#L61

uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;

Impact:

This flaw can cause under-distribution of rewards, leaving some tokens trapped within the contract. Since these remaining tokens are not accounted for or redistributed, they become inaccessible, potentially causing a loss of funds for the intended recipients (claimants). Furthermore, the manager's cut and the claimants' rewards are both incorrectly calculated, reducing the expected rewards for the claimants.

Proof of Concept:

  1. Deploy the contract with a set of players and their corresponding rewards.

  2. Simulate a scenario where only a subset of players claims their rewards.

  3. Call the closePot function after 90 days.

  4. Observe that the claimants receive a lower amount than expected because the remaining rewards are divided by the total number of players instead of the actual number of claimants.

  5. Check the contract balance afterward to confirm that leftover tokens remain trapped within the contract.

Recommended Mitigation:

Update the closePot function to calculate claimantCut based on the number of actual claimants instead of the total number of players. Additionally, ensure that any remainder from integer division is distributed appropriately, either by allocating it to the manager or the final claimant to prevent tokens from being trapped in the contract.

Example Fix:

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 totalClaimants = claimants.length;
uint256 claimantCut = (remainingRewards - managerCut) / totalClaimants;
uint256 remainder = remainingRewards - managerCut - (claimantCut * totalClaimants);
​
for (uint256 i = 0; i < totalClaimants; i++) {
if (i == totalClaimants - 1) {
_transferReward(claimants[i], claimantCut + remainder); // Last claimant gets the remainder
} else {
_transferReward(claimants[i], claimantCut);
}
}
}
}

This solution ensures that the rewards are correctly distributed among the claimants, with no leftover tokens remaining in the contract.

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.