MyCut

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

Loss of funds when closing the pot with 0 claimants

Summary

Pot contract allows authorized claimants 90 days to claim before the manager takes a cut of the remaining pool and the remainder is distributed equally to those who claimed in time. However, in the scenario where there are 0 claimants, protocol will loose funds and they will remain locked in the Potcontract.

Vulnerability Details

When closing the pot the following function is called:

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);
}
}
}

This function is used to distribute rewards to the claimants who claimed in time (90 days from contest start). However, if there are 0 claimants when the closePot function is called, only 10% of the total funds could be restored.

Proof of Conept:

Place the following test in the test/TestMyCut.t.sol:

function testFundsLockedIfNoClaimers() public mintAndApproveTokens {
vm.startPrank(user);
//added more players
rewards = [220, 230, 500, 50];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
assert(ERC20Mock(weth).balanceOf(contest) == totalRewards - totalRewards/10);
//balanceOf(conMan) since it is the (wrong) owner, not the user
assert(ERC20Mock(weth).balanceOf(conMan) == totalRewards/10);
}

Impact

This vulnerability will cause the loss of funds. The funds will remain locked in the Pot contract, as there is no option to withdraw them.

Tools Used

Manual code review / Foundry tests

Recommendations

Consider a scenario where there are 0 claimants, and in that scenario, transfer all the funds to the Pot owner.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect handling of zero claimant edge case

Support

FAQs

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