[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(user), (1000e18 - 15e18) + 0.4 ether);
assertEq(weth.balanceOf(players_[1]), 1327272727272727272);
assertEq(weth.balanceOf(contest), 8);
}
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)));
}
}