Submission Details
Severity:
**[H-4] No validation on `totalRewards` being equal to the sum of elements in `rewards`**
**Description:** `totalRewards` is being manually passed in as a constructor parameter of `ContestManager`, there is no validation that `totalRewards == sum of the rewards array elements (rewardsSum)` There are two particular cases of interest:
- 1) `rewardsSum > totalRewards`: This scenario will lead to the Pot being under funded and unable to make payouts to users who attempt to claim their cut. There will be an arithmetic underflow when the protocol is unable to payout a particlar user within `claimCut`.
```solidity
function claimCut() public {
// Follows CEI.
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
// audit: q - reward is a uint256 it's minimum value is 0, it won't be less than 0.
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward; // <-- Here we expect an underflow
claimants.push(player);
_transferReward(player, reward); // audit: q - This will revert is we have not funded the pot, perhaps a boolean state variable to check if funded or balance check.
}
```
- 2) `rewardsSum < totalRewards`: Users will be able to claim their cut, however the protocol will also be distributing additional funds to all claimants as there will be a surplus of the `ERC20` token used in the contest.
**Impact:** The two scenarios described above may result in users who cannot be paid out or users that have claimed their cut being paid an additional amount when the contest manager closes the pot.
**Proof of Concept:**
Add the below test functions to `TestMyCut.t.sol:TestMyCut` contract test suite.
```solidity
/**
* Users will be able to claim their cut in addition to gaining a surplus amount of funds
* when the contest is closed.
*/
function testTotalRewardsGreaterThanRewardsElementsSum() public mintAndApproveTokens {
// Arrange - Let both users claim their cut.
totalRewards = 100;
rewards = [40, 40];
ContestManager contestManager = ContestManager(conMan);
vm.startPrank(user);
contest = contestManager.createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
contestManager.fundContest(0);
vm.stopPrank();
Pot contestPot = Pot(contest);
for (uint256 i = 0; i < players.length; i++) {
vm.prank(players[i]);
contestPot.claimCut();
}
uint256 playerOnePostClaimBalance = weth.balanceOf(players[0]);
uint256 playerTwoPostClaimBalance = weth.balanceOf(players[1]);
// Act - Show the users receive additional funds when the pot is closed, despite having claimed already.
vm.warp(91 days);
vm.prank(user);
contestManager.closeContest(contest);
uint256 playerOneBalancePostPotClosure = weth.balanceOf(players[0]);
uint256 playerTwoBalancePostPotClosure = weth.balanceOf(players[1]);
assertTrue(playerOneBalancePostPotClosure > playerOnePostClaimBalance);
assertTrue(playerTwoBalancePostPotClosure > playerTwoPostClaimBalance);
}
/**
* Users may not be able to claim their cut.
*/
function testTotalRewardsLessThanRewardsElementsSum() public mintAndApproveTokens {
// Arrange
totalRewards = 20;
rewards = [20, 20];
ContestManager contestManager = ContestManager(conMan);
vm.startPrank(user);
contest = contestManager.createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
contestManager.fundContest(0);
vm.stopPrank();
// Act - Player 1 can claim but player 2 will be unable to claim their cut.
Pot contestPot = Pot(contest);
vm.prank(player1);
contestPot.claimCut();
// Assert - Expect reversion (Arithmetic underflow)
vm.expectRevert();
vm.prank(player2);
contestPot.claimCut();
}
```
**Recommended Mitigation:**
Although slightly more gas-inefficient, it is worth the additional gas then to have a mismatch between the sum of the element of the `rewards` array and the `totalRewards`.
```diff
- function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
+ function createContest(address[] memory players, uint256[] memory rewards, IERC20 token)
public
onlyOwner
returns (address)
{
+ uint256 totalRewards;
+ for (uint256 i=0; i<rewards.length; i++) {
+ totalRewards += rewards[i];
+ }
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
```
Updates
Rewards breakdown
nSLOC
106This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.