MyCut

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

Dust Amounts Will Be Left After Calling `closeContest`

[L-01] Dust Amounts Will Be Left After Calling closeContest

Summary

Due to rounding issues in the calculation of claimantCut, some dust amounts will be left in the contract after calling the closePot function. The rounding error occurs because the division of (remainingRewards - managerCut) by claimants.length can result in a small remainder that cannot be distributed to claimants and thus gets stuck in the contract.

Vulnerability Details

The order of calculations for managerCut and claimantCut is incorrect. Because of rounding issues when dividing (remainingRewards - managerCut) by claimants.length, some small amounts (dust) will remain in the contract after distributing rewards to claimants.
Also note that instead of i_players.length the claimants.length is used, this bug has been reported speratly.

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) /
@> claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

Impact

Dust amounts of the reward token will be left in the contract and cannot be retrieved or used.

Proof of Concept

Add the following test to the existing test suite to reproduce the issue:

function test_my_DustAmountStuck() public mintAndApproveTokens {
address[] memory players_ = new address[](15);
uint256[] memory rewards_ = new uint256[](15);
for (uint256 i; i < 15; i++) {
players_[i] = address(uint160(i + 10));
rewards_[i] = 1 ether;
}
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
players_,
rewards_,
IERC20(ERC20Mock(weth)),
15 ether
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
for (uint256 i; i < 11; i++) {
vm.prank(players_[i]);
Pot(contest).claimCut();
}
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
// assertEq(weth.balanceOf(conMan), 0.4 ether); //bug 1
assertEq(weth.balanceOf(user), (1000e18 - 15e18) + 0.4 ether);
assertEq(weth.balanceOf(players_[1]), 1327272727272727272);
assertEq(weth.balanceOf(contest), 8); // 8 wei stuck in the pot!
// 15 - 11 = 4 ether left
// 4 / 10 = 0.4
// 11 have claimed, 4-0.4 = 3.6 is left, 3.6/11 = 3.272727... + 1 ether from before
}

Tools Used

Manual Review

Recommendations

Reverse the order and send the remaining dust amount to the manager. This ensures that any remaining balance is accounted for and appropriately transferred.

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) /
claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
+ i_token.transfer(msg.sender, i_token.balanceOf(address(this)));
}
}
Updates

Lead Judging Commences

equious Lead Judge 12 months 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.