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

H-01: User Can Claim Rewards Even After Contest Closure

syedrabeet2002

Summary

The vulnerability allows users to claim rewards using the claimCut function even after the contest has been closed using closeContest function, bypassing the intended restriction on post-closure claims.

Vulnerability Details

The vulnerability stems from the lack of a proper state check in the claimCut function of the Pot.sol contract. The contest is intended to close after a specified period (e.g., 90 days), at which point no further reward claims should be possible. However, the claimCut function does not verify whether the contest is still open before allowing users to claim their rewards.

function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
_transferReward(player, reward);
}
​

Add this test case in your TestMyCut.t.sol file

function testCanClaimRewardAfterTheContestIsClosed() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
uint256 claimantBalanceBefore = ERC20Mock(weth).balanceOf(player1);
​
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
uint256 claimantBalanceAfter = ERC20Mock(weth).balanceOf(player1);
​
assert(claimantBalanceAfter > claimantBalanceBefore);
}

In this scenario, Player 1 is able to claim a reward after the contest is closed, which should not be possible under the correct functionality.

Impact

  • Users can claim rewards even after the contest has ended, bypassing the intended restriction and potentially accessing funds they shouldn’t be entitled to.

  • The remaining rewards, which should be managed or redistributed by the contract, might be depleted by unauthorized claims, leading to a financial loss for the contract owner or other stakeholders.

  • Claims made after closure can distort the fairness of reward distribution, leading to discrepancies where some participants might receive more than their fair share or others might receive less.

Tools Used

Manual Review

Recommendations

Please incorporate the following code into the Pot.sol contract. I have introduced a boolean variable, isOpen, which is set to true upon contract deployment and changed to false when the contest is closed. This ensures that any attempts to claim rewards after the contest has closed will result in a transaction revert with the error Pot__IsClosed().

This update ensures that reward claims are only processed if the contest is still open, and prevents any claims after the contest has officially closed.

+++ error Pot__IsClosed();
​
+++ bool private isOpen;
​
constructor(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
+++ isOpen = true;
// i_token.transfer(address(this), i_totalRewards);
​
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
​
function claimCut() public {
+++ if (!isOpen) {
+++ revert Pot__IsClosed();
+++ }
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
_transferReward(player, reward);
}
​
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);
}
}
​
+++ isOpen = false;
}
Updates

Lead Judging Commences

equious Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Appeal created

syedrabeet2002 Submitter
9 months ago
equious Lead Judge
9 months ago
equious Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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