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

Owner might mistakenly fund a contest that has already ended , causing them to lose out on their funds

SoulReaper

Summary

Owner might mistakenly fund a contest that has already ended , causing them to lose out on their funds

Vulnerability Details

In the ContestManager contract , owner has functionality to close an existing pot. But after a pot closes , owner can still fund that contract (using ContestManager::fundContest). The owner would obviously not do this on purpose , but if they do , they have no way to get all those funds back. Only thing they can do is call the Pot::closePot function. But this function only gives the owner 10% of the total rewards , so the owner will lose out on 90% of their funds.

One more problem is that Pot::claimCut has no controls to prevent users from claiming if the pot has ended. If the user didn't claim before owner called closePot , then this user shouldn't be able to claim afterwards. In normal circumstances when the pot is funded only once , this functionality is preserved as even if this user tries to claim , claimCut would revert as the contract wouldn't have funds(actually it would due to another bug in closePot , but let's ignore that for now). Now if the owner funds the pot again , this user can claim. Now the contract balance is less than Pot::i_totalRewards , and now if the owner calls closePot , they will get less than 10% if the total rewards , which is even worse

This description got a little messy since 3 bugs are into play , but to summarise:
The owner may accidently fund a pot that has already closed , leading to them loosing 90% or more of the funded amount.

Impact

Owner will loose money if they fund a closed pot.

Proof of Concepts

  1. Owner creates and funds a pot

  2. Player 1 claims his reward

  3. Owner closes the pot

  4. Owner funds it again

  5. Player 2 claims

  6. Owner closes the pot again but doesnt get all of their funds back

Place this into TestMyCut.t.sol

function test_FundingOfContractAfterItHasEnded() public mintAndApproveTokens{
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 4);
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();
​
// now fund it again
​
vm.prank(user);
ContestManager(conMan).fundContest(0);
​
uint256 balanceBefore = IERC20(ERC20Mock(weth)).balanceOf(player2);
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
​
uint256 balanceAfter = IERC20(ERC20Mock(weth)).balanceOf(player2);
​
assert(balanceAfter - balanceBefore == 1);
​
​
uint256 userBalanceBefore = IERC20(ERC20Mock(weth)).balanceOf(user);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
uint256 userBalanceAfter = IERC20(ERC20Mock(weth)).balanceOf(user);
​
assert(userBalanceAfter - userBalanceBefore < totalRewards);
​
}

Tools Used

Manual Review , Foundry Tests

Recommendations

Keep track of which contests have ended , so the owner cannot fund a closed pot.
Make a mapping of address to boolean for the same.

+ mapping(address pot => bool isClosed) public isClosed;
+ error ContestManager__CannotFundClosedContest();
.
.
.
​
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
+ if(isClosed[address(pot)]){
+ revert ContestManager__CannotFundClosedContest();
+ }
.
.
.
}
.
.
.
​
function closeContest(address contest) public onlyOwner {
+ if(!isClosed[contest]){
+ isClosed[address(pot)] = true;
+ _closeContest(contest);
+ }
- _closeContest(contest);
}
Updates

Lead Judging Commences

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

Support

FAQs

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