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

`remainingRewards` is not properly cleared after reward distribution

cryptolappi

Summary

A significant vulnerability has been identified in the Pot smart contract where the remainingRewards state variable is not properly cleared after reward distribution, potentially leading to inaccurate reward reporting and inconsistent contract state.

Vulnerability Details

In the Pot contract:

uint256 private remainingRewards;
​
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);
}
}
// remainingRewards is not set to 0 after distribution
}

The remainingRewards variable is not reset to zero after distributing all rewards in the closePot function.

Impact

  1. Inaccurate State: The contract may report non-zero remaining rewards even after all rewards have been distributed, leading to a misrepresentation of the contract's financial state.

  2. Potential for Double Spending: If the contract is reused or if there are functions that rely on remainingRewards, it could lead to attempts to distribute rewards that no longer exist.

  3. Misleading Information: Functions like getRemainingRewards() would return incorrect values, potentially misleading users or other contracts that interact with this contract.

  4. Inconsistent Internal State: The discrepancy between remainingRewards and the actual token balance of the contract could lead to unexpected behaviors in future transactions or contract upgrades.

Tools Used

Manual code review

AI for report

Recommendations

  1. Clear Remaining Rewards: Update the closePot function to clear remainingRewards:

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);
}
remainingRewards = 0; // Clear remaining rewards
}
}

  1. Add Safety Checks: Implement additional checks to ensure remainingRewards is consistent with the sum of unclaimed rewards:

function validateRemainingRewards() internal view {
uint256 calculatedRemaining = 0;
for (uint256 i = 0; i < i_players.length; i++) {
calculatedRemaining += playersToRewards[i_players[i]];
}
require(calculatedRemaining == remainingRewards, "Inconsistent remaining rewards");
}
Updates

Lead Judging Commences

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

Support

FAQs

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