MyCut

First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Invalid

Storage variable remainingRewards is not being updated

Summary

The variable remainingRewards, which is used in the calculation of the distributed rewards, is not updated in closePot function.

Vulnerability Details

The remainingRewards variable is not updated within the closePot() function after performing reward distribution calculations. This discrepancy occurs because the remainingRewards variable is only updated within the claimCut() function. As a result, users can call the getRemainingRewards function and receive an outdated value for remainingRewards, reflecting the state before the closePot() function was executed. This leads to inconsistency in the storage variable, causing potential confusion or incorrect assumptions about the remaining rewards in the contract.

function closePot() external onlyOwner {
....
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 failure to update the remainingRewards variable within the closePot() function results in inconsistent storage data, which can mislead users regarding the actual amount of remaining rewards in the contract.

Tools Used

PoC

function test_close_pot_after_90_days() public mintAndApproveTokens {
rewards = [1000 wei,2000 wei,3000 wei,4000 wei,5000 wei,6000 wei];
totalRewards = 21000 wei;
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
vm.warp(block.timestamp + 91 days);
uint256 remainingRewards = Pot(contest).getRemainingRewards();
assert(remainingRewards == totalRewards);
uint256 balanceBeforePlayer1 = ERC20Mock(weth).balanceOf(player1);
uint256 balanceBeforePlayer2 = ERC20Mock(weth).balanceOf(player2);
assertEq(balanceBeforePlayer1, 0);
assertEq(balanceBeforePlayer2, 0);
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
uint256 balanceAfterPlayer1 = ERC20Mock(weth).balanceOf(player1);
uint256 balanceAfterPlayer2 = ERC20Mock(weth).balanceOf(player2);
// claimantCut = 8100
assertEq(balanceAfterPlayer1, 9100);
assertEq(balanceAfterPlayer2, 10100);
// remainingRewards did not changed, after closeContest was called
uint256 remainingRewardsFirstClose = Pot(contest).getRemainingRewards();
assertEq(remainingRewardsFirstClose, 18000);
}

Recommendations

Consider updating the remainingRewards variable inside of the closePot() function.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Appeal created

tm2 Submitter
about 1 year ago
equious Lead Judge
about 1 year ago
equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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