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

Potential erroneous calculation of the Manager's cut in `Pot::closePot` , causing the manager to lose some funds

SoulReaper

Summary

Potential erroneous calculation of the Manager's cut in Pot::closePot , causing the manager to lose some funds

Vulnerability Details

The calculation of the manager's cut in closePot is as follows:

uint256 managerCut = remainingRewards / managerCutPercent;

Now , managerCutPercent is intended to be 'how much pecent of the remaining rewards should the manager get' . This value is hardcoded to be 10 in the Pot contract. Now see , 10% means 1/10 so remainingRewards/10 gives the cut of the manager. But if the developers decide to change this fee percentage to, say 15 , then this formula will not work.

The owner will expect (15 * remainingRewards)/100 as his cut , but he will get remainingRewards/15 ~ 6.67% of the remainingRewards.
Clearly the owner will lose out on his cut and get way less (or way more , depending on value of managerCutPercent) than expected

Proof of Concepts

  1. (Change managerCutPercent to 15 for this test)

  2. Owner creates and funds the pool

  3. Player 1 claims

  4. Deadline passes

  5. Owner calls closePot

  6. Owner expects 15 % of remainingRewards

  7. Owner gets 6.67% of remainingRewards

Change managerCutPercent to 15 for this test

Place this into TestMyCut.t.sol

function test_ClosePotHasWrongMath_2() 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();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
uint256 ownerBalanceBefore = ERC20Mock(weth).balanceOf(conMan);
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
uint256 ownerBalanceAfter = ERC20Mock(weth).balanceOf(conMan);
​
// assert(ownerBalanceAfter - ownerBalanceBefore == 75); // expects 15 % of 500
assert(ownerBalanceAfter - ownerBalanceBefore == 33); // gets 6.67% of 500 (1/15 == 0.0667)
}

Impact

Owner will get less cut in some cases (managerCutPercent > 10)

Tools Used

Manual Review , Foundry

Recommendations

Change the formula as follows

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
- uint256 managerCut = remainingRewards / managerCutPercent;
+ uint256 managerCut = (managerCutPercent * remainingRewards)/100;
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);
}
}
}
Updates

Lead Judging Commences

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

Dusty Pot

Support

FAQs

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