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

`Pot::remainingRewards` Not Updated in `Pot::closePot` Function

Dec3mber

Summary

The Pot::remainingRewards variable is not updated within the Pot::closePot function, leading to potential inconsistencies between the actual state of the contract and what is reported by the Pot::getRemainingRewards function. Whether i_token.transfer(msg.sender, managerCut); or _transferReward(claimants[i], claimantCut); transcation fails or not, you can't get the right result through the Pot::getRemainingRewards function.

Vulnerability Details

In the Pot contract, the Pot::remainingRewards variable tracks the amount of unclaimed rewards left in the pot. However, in the Pot::closePot function, where rewards are distributed to the manager and claimants, the Pot::remainingRewards is not updated after these transactions. This oversight can cause the Pot::getRemainingRewards function to return an outdated or incorrect value:

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);
@> // Missing update to remainingRewards after giving manager cut
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
​
@> // Missing update to remainingRewards after distribution
}
}

Without updating Pot::remainingRewards to reflect the actual distribution, the value returned by Pot::getRemainingRewards may incorrectly indicate that there are still funds available in the pot, even after it has been closed and the funds have been distributed.

Impact

  • Incorrect Reward Reporting: Users may be misled by incorrect values returned by Pot::getRemainingRewards, believing there are still rewards to be claimed when in fact the pot has already been closed and funds distributed.

  • Inconsistent Contract State: The contract state may become inconsistent, leading to potential issues in future transactions or audits, as the actual remaining rewards do not match what is reported.

  • Potential Mismanagement: Inaccurate reporting of remaining rewards can lead to the mismanagement of funds, where further actions are taken based on incorrect data

Tools Used

Manual Review

Recommendations

To resolve this issue, ensure that Pot::remainingRewards is correctly updated after distributing rewards in the Pot::closePot function:

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);
+ remainingRewards -= managerCut;
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
+ remainingRewards -= claimantCut;
}
​
+ remainingRewards = 0; // Ensure remainingRewards is updated after all distributions
}
}

This change ensures that Pot::remainingRewards accurately reflects the current state of the contract after the pot has been closed and all funds have been distributed.

Updates

Lead Judging Commences

equious Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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