Root + Impact
The root cause is that closePot() transfers rewards without updating remainingRewards or recording that the pot has already been closed.
Description
The remainingRewards variable is not reduced after the transfers. Consequently, every subsequent call uses the original remaining-reward amount to calculate payouts again.
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);
}
}
}
Risk
Likelihood: The issue is directly reachable by the owner after 90 days, although exploitation depends on the owner making repeated closure calls.
Impact: Repeated payouts can exceed the intended allocation and exhaust the pot's token balance, potentially leaving other expected payments unfunded.
Proof of Concept
The following Foundry test demonstrates the repeated-payout behavior using the project's existing contracts and test setup.
function testClosePotCanDistributeRewardsRepeatedly()
public
{
address[] memory players = new address[](1);
players[0] = address(0x1234);
uint256[] memory rewards = new uint256[](1);
rewards[0] = 1000;
uint256 totalRewards = 1000;
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
players,
rewards,
IERC20(weth),
totalRewards
);
ERC20Mock(weth).mint(user, totalRewards);
ERC20Mock(weth).approve(conMan, totalRewards);
ContestManager(conMan).fundContest(0);
vm.warp(block.timestamp + 91 days);
uint256 managerBalanceBefore = weth.balanceOf(user);
ContestManager(conMan).closeContest(contest);
uint256 managerBalanceAfterFirstClose =
weth.balanceOf(user);
ContestManager(conMan).closeContest(contest);
uint256 managerBalanceAfterSecondClose =
weth.balanceOf(user);
vm.stopPrank();
assertGt(
managerBalanceAfterFirstClose,
managerBalanceBefore
);
assertGt(
managerBalanceAfterSecondClose,
managerBalanceAfterFirstClose
);
}
Expected result: The second closure call succeeds if the pot still has sufficient tokens, and the manager receives another payout calculated from the unchanged remainingRewards.
Recommended Mitigation
Make the closure operation execute only once. Add a closed-state flag and update the accounting before making external token transfers. Revert any subsequent closure attempts.
Make the closure operation execute only once. Add a closed-state flag and update the accounting before making external token transfers. Revert any subsequent closure attempts.
Implementation note: The mitigation above illustrates the required protections, but the production implementation should also handle an empty claimants array, account for rounding dust, and ensure that the distribution formula matches the intended contest rules. Updating accounting and adding a closed-state check prevents repeated closure payouts.
contract Pot is Ownable(msg.sender) {
+ bool public closed;
function closePot() external onlyOwner {
+ require(!closed, "Pot already closed");
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
+ closed = true;
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
+ uint256 rewardsToDistribute = remainingRewards;
+ remainingRewards = 0;
i_token.transfer(msg.sender, managerCut);
- uint256 claimantCut =
- (remainingRewards - managerCut) / i_players.length;
+ uint256 claimantCut =
+ (rewardsToDistribute - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
}
