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

claimCut function can be called even after closing the pot

0xiceman

Summary

Protocol docs specify that the reward can be claimed in the first 90 days. However, the Pot contract allows players to call the claim even after the pot has been closed (even tough there is possibly 0 rewards remaining).

Vulnerability Details

The claimCut function remains callable even after the closePot function has been executed since there is no check to prevent this.

Proof of Concept:

function testCanClaimAfterClosingThePot() public mintAndApproveTokens {
vm.startPrank(user);
//add more players to demontstrate this
rewards = [220, 230, 500, 50];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
uint256 playerBalanceBefore = ERC20Mock(weth).balanceOf(player4);
//balanceOf(player4) will increase due to incorrect implementation of the close pot function
//that fact is used to demonstrate this vulnerability :)
vm.startPrank(player4);
Pot(contest).claimCut();
vm.stopPrank();
​
​
uint256 playerBalanceAfter = ERC20Mock(weth).balanceOf(player4);
assert(playerBalanceBefore + 50 == playerBalanceAfter);
}

Impact

While players can call the claimCut function after pot closure, the actual transfer of tokens will likely revert if the contract's balance is less than the amount the user is trying to claim, or pass if contract balance is suffitient. This can lead to gas wastage, a poor user experience and unintended behaviour of the protocol.

Tools Used

Manual code review / Foundry tests

Recommendations

  1. Implement a closed state: Add a boolean state variable to track whether the pot has been closed:

bool private isPotClosed;
​
function closePot() external onlyOwner {
require(!isPotClosed, "Pot is already closed");
// ... existing closing logic ...
isPotClosed = true;
}

  1. Prevent claims after closure: Modify the claimCut function to check the closed state:

function claimCut() public {
require(!isPotClosed, "Pot is closed, no more claims allowed");
// ... existing claim logic ...
}

  1. Set the remaining rewards to 0 after closing the Pot:

function closePot() external onlyOwner {
if (block.timestamp - i\_deployedAt < 90 days) {
revert Pot\_\_StillOpenForClaim();
}
if (remainingRewards > 0) {
(...)
remainingRewars = 0;
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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