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

Potential Token Loss (Due to Integer Division Rounding and Unclaimed Rewards)

princekay

Potential Token Loss (Due to Integer Division Rounding and Unclaimed Rewards)

Description:

The closePot function in the contract is susceptible to potential token loss because of integer division rounding errors and the handling of unclaimed rewards. When distributing the remaining rewards, the function calculates the managerCut and claimantCut using integer division, which can lead to truncation and loss of token amounts due to rounding down. Additionally, if there are no claimants or if division errors result in tokens not being fully distributed, these tokens can remain trapped in the contract. Currently, there is no mechanism in place to recover or redistribute these stuck tokens, potentially leading to permanent loss.

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

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 combined effect of rounding errors and unclaimed rewards can result in tokens being permanently stuck in the contract, causing a loss of funds. This reduces the effectiveness of the reward distribution and impacts both claimants and the manager, as some of the intended rewards are never actually distributed. The trapped tokens become irrecoverable, which diminishes the overall trustworthiness and efficiency of the contract.

Proof of Concept:

  1. Deploy the contract with a specific total rewards pool.

  2. Simulate the scenario where:

    • No players claim their rewards.

    • Only a small subset of players claim their rewards.

  3. After 90 days, call the closePot function.

  4. Observe that due to integer division, small amounts of tokens are not distributed to claimants or the manager and remain stuck in the contract.

  5. Check the remaining balance in the contract after the function execution, which will show that there are tokens trapped due to rounding errors and unclaimed rewards.

Recommended Mitigation:

To prevent token loss, implement a mechanism that properly handles rounding errors and unclaimed rewards. This can be achieved by:

  1. Adjusting the Distribution Logic: Ensure that any remainder from the integer division after calculating the claimantCut is allocated to the last claimant or to the manager. This prevents tokens from being trapped due to rounding.

  2. Fallback Mechanism: Introduce a fallback mechanism that allows the contract owner to recover or redistribute any tokens that remain in the contract after the initial distribution. This ensures that no tokens are permanently stuck.

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;
if (totalClaimants > 0) {
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);
}
}
} else {
// If no claimants, transfer all remaining rewards to the manager or revert the transaction.
i_token.transfer(msg.sender, remainingRewards);
}
}
}

This approach ensures that all remaining tokens are either fully distributed among the claimants or recovered by the manager, leaving no tokens trapped within the contract.

Updates

Lead Judging Commences

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

Incorrect handling of zero claimant edge case

Dusty Pot

Support

FAQs

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