Summary
The Pot::closePot function can be called multiple times, leading to the distribution of more tokens to players and the manager than intended if the contract has ended.
Vulnerability Details
The closePot function in the Pot contract allows the owner to close the pot and distribute the remaining rewards. However, there is no mechanism in place to prevent this function from being called multiple times. As a result, each call to closePot will distribute the remaining rewards again, potentially leading to over-distribution.
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);
}
}
}
Proof Of Concept
After the pot has ended (90 days have passed), the owner can call the closePot function multiple times, leading to over-distribution of the remaining rewards.
function testCanClosePotMultipleTimes() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [200, 200];
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 400);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
vm.warp(91 days);
uint256 initialBalanceManager = ERC20Mock(weth).balanceOf(conMan);
uint256 initialBalancePlayer1 = ERC20Mock(weth).balanceOf(player1);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
uint256 newBalanceManager = ERC20Mock(weth).balanceOf(conMan);
uint256 newBalancePlayer1 = ERC20Mock(weth).balanceOf(player1);
assert(newBalanceManager > initialBalanceManager);
assert(newBalancePlayer1 > initialBalancePlayer1);
vm.startPrank(user);
ContestManager(conMan).fundContest(0);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
assert(ERC20Mock(weth).balanceOf(conMan) > newBalanceManager);
assert(ERC20Mock(weth).balanceOf(player1) > newBalancePlayer1);
}
Impact
Solidity compiler
Manual code review
Foundry
If the closePot function is called more than once, it will result in the Pot contract distributing more tokens than intended. This can lead to:
Over-distribution of rewards to players and the manager.
Potential depletion of the contract's token balance.
Discrepancies in the intended reward distribution.
Tools Used
Solidity compiler
Manual code review
Foundry
Recommendations
To mitigate this vulnerability, implement a mechanism to ensure that the closePot function can only be called once. This can be achieved by adding a closed status check within the function. Here is an updated version of the Pot contract with the recommended changes:
pragma solidity ^0.8.20;
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {Ownable} from "lib/openzeppelin-contracts/contracts/access/Ownable.sol";
contract Pot is Ownable(msg.sender) {
error Pot__RewardNotFound();
error Pot__InsufficientFunds();
error Pot__StillOpenForClaim();
error Pot__AlreadyClosed();
address[] private i_players;
uint256[] private i_rewards;
address[] private claimants;
uint256 private immutable i_totalRewards;
uint256 private immutable i_deployedAt;
IERC20 private immutable i_token;
mapping(address => uint256) private playersToRewards;
uint256 private remainingRewards;
uint256 private constant managerCutPercent = 10;
bool private s_isClosed;
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
s_isClose = false;
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
function closePot() external onlyOwner {
if (s_isClose) {
revert Pot__AlreadyClosed();
}
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);
}
remainingRewards = 0;
}
s_isClose = true;
}
}
