Impact: High
Likelihood: High
Root + Impact
Description
-
The protocol's invariant remainingRewards should be equal to pot balance proves a protocol solvency.
-
When a Pot is created, the value of remainingRewards is assigned equal to totalRewards. When a Pot is funded, exactly totalRewards amount is transferred to Pot.sol. This means that remainingRewards reflects balanceOf(pot) and these two values should be synchronized.
-
A pot may be funded several times, but remainingRewards is not synchronized with a new balance.
-
remainingRewards value is not assigned to zero or synchronized with a pot balance when Pot::closePot() function is called.
Pot.sol constructo assignes remainingRewards equal to totalRewards in constructor:
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;
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
ContestManager::fundContest transfers totalRewards and sets a Pot.sol balance:
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
@> token.transferFrom(msg.sender, address(pot), totalRewards);
}
remainingRewards is not zeroed or synchronized with a pot balance, allowing it to be more than balance and causing a protocol insolvency:
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 occurs at the very beginning of the contest, when not-funded pot already has remainingRewards which means that a contest is ready to be started, but it was not funded yet and it is insolvent.
-
Once a pot is funded, the invariant remainingRewards should be equal to pot balance holds till a Pot::closePot() function is called - remainingRewards is never zeroed or synchronized with a pot balance.
Impact:
Proof of Concept
Values of remainingRewards and Pot.sol balance are not equal during a protocol lifecycle.
Please, add the following test to TestMyCut.t.sol:
function test_remainingRewardsIsNotEqualToPotBalance() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
uint256 potBalInitial = ERC20Mock(weth).balanceOf(contest);
uint256 remainingRewardsInitial = Pot(contest).getRemainingRewards();
assertGt(remainingRewardsInitial, potBalInitial);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
uint256 potBalFunded = ERC20Mock(weth).balanceOf(contest);
uint256 remainingRewardsFunded = Pot(contest).getRemainingRewards();
assertEq(potBalFunded, remainingRewardsFunded);
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
uint256 potBalClaimed = ERC20Mock(weth).balanceOf(contest);
uint256 remainingRewardsClaimed = Pot(contest).getRemainingRewards();
assertEq(potBalClaimed, remainingRewardsClaimed);
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
uint256 potBalFinal = ERC20Mock(weth).balanceOf(contest);
uint256 remainingRewardsFinal = Pot(contest).getRemainingRewards();
assertGt(remainingRewardsFinal, potBalFinal);
}
Recommended Mitigation
It is recommended to synchronize remainingRewards with a pot balance and account additionally transferred funds in Pot::remainingRewards:
-uint256 private remainingRewards;
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;
// i_token.transfer(address(this), i_totalRewards);
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
+ uint256 remainingRewards = i_token.balanceOf(address(this));
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);
}
}
}
