MyCut

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

**[M-2] `remainingRewards` is not set to 0 after closing the pot. May result in additional funds being sent to claimants and the manager.**

**Description:** The `closePot` function does not set `remainingRewards` to zero. Therefore if the `closePot` function were to be called my the `contestManager` the `remainingRewards` will be greater than 0 and execute the logic associated with determining the `managersCut` and splitting funds amongst `claimants` even though the logic had already been performed upon the initial closure of the pot.
```solidity
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
// audit: q - magic number hard-coded.
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) { // This needs to be set to 0 at the end of the function.
uint256 managerCut = remainingRewards / managerCutPercent; // audit: q - Division is throwing me off, SafeMath? This can go wrong leading to precision loss.
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length; // audit: q - We are splitting amongst the claimaints, use the claimaints lenth not i_players.
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
```
**Impact:** Pot can be closed multiple times and an attempt at transferring tokens to the manager and the claimants will occur again, I believe this results in the protocol incorrectly closing the pot.
**Proof of Concept:**
The below test can be added to `TestMyCut.t.sol:TestMyCut` contracts test suite.
```solidity
/**
* Demonstrating that the pot can be closed multiple times
* as remainingRewards was not set to 0.
*/
function testRemainingRewardsNotSetToZeroPostPotClosure() public mintAndApproveTokens {
// Arrange
rewards = [500, 500];
totalRewards = 1000;
vm.startPrank(user);
ContestManager contestManager = ContestManager(conMan);
contest = contestManager.createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
contestManager.fundContest(0);
vm.stopPrank();
Pot pot = Pot(contest);
vm.prank(players[0]);
pot.claimCut();
uint256 remainingFundsAfterFirstClaim = pot.getRemainingRewards();
// Act - We would expect the remaining funds to be sent to the
vm.warp(91 days);
vm.startPrank(user);
contestManager.closeContest(contest);
uint256 remainingFundsAfterPotClosed = pot.getRemainingRewards();
// Assert
assertEq(remainingFundsAfterFirstClaim, remainingFundsAfterPotClosed);
assertFalse(remainingFundsAfterPotClosed == 0);
}
```
**Recommended Mitigation**:
```diff
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;
}
```
Updates

Lead Judging Commences

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

Appeal created

0xNascosta 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.