MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Calculation of Claimant Cut in `Pot::closePot`

esoetheric

Summary

The Pot::closePot function calculates the amount to be distributed among the claimants using i_players.length instead of claimants.length. This results in an incorrect distribution of the remaining rewards, potentially locking funds in the contract.

Impact

This issue can lead to an under-distribution of the remaining rewards to claimants, leaving a portion of the funds locked in the contract. This is particularly critical in scenarios with many claimants, where the miscalculation could result in significant locked funds.

Tools Used

Manual Review

Proof of Concept

The following variables should be added to TestMyCut.t.sol:

address[] testPlayers = [
address(uint160(1)),
address(uint160(2)),
address(uint160(3)),
address(uint160(4)),
address(uint160(5)),
address(uint160(6)),
address(uint160(7)),
address(uint160(8)),
address(uint160(9)),
address(uint160(10))
];
uint256[] testRewards = [
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether,
2 ether
];
uint256 testTotalRewards = 20 ether;

Then, add the following test to the existing tests in TestMyCut.t.sol:

function testIncorrectClosure() public mintAndApproveTokens {
vm.startPrank(user);
ContestManager testConMan = ContestManager(conMan);
​
address testPot = testConMan.createContest(
testPlayers,
testRewards,
IERC20(weth),
testTotalRewards
);
testConMan.fundContest(0);
vm.stopPrank();
console.log("Initial Pot Balance: ", weth.balanceOf(address(testPot)));
uint256 numberOfClaimers = 5;
for (uint i; i < numberOfClaimers; i++) {
vm.prank(testPlayers[i]);
Pot(testPot).claimCut();
}
console.log(
"Pot Balance after Claim: ",
weth.balanceOf(address(testPot))
);
uint256 potBalance = weth.balanceOf(address(testPot));
vm.warp(block.timestamp + 100 days);
​
console.log(
"Owner Balance before Closure: ",
weth.balanceOf(address(user))
);
console.log(
"Player Balance before Closure: ",
weth.balanceOf(address(testPlayers[0]))
);
uint256 playerBalanceBeforeClosure = weth.balanceOf(
address(testPlayers[0])
);
console.log(
"ContestManager Balance before Closure: ",
weth.balanceOf(address(conMan))
);
vm.prank(user);
testConMan.closeContest(testPot);
​
console.log(
"Owner Balance after Closure: ",
weth.balanceOf(address(user))
);
console.log(
"ContestManager Balance after Closure: ",
weth.balanceOf(address(conMan))
);
console.log(
"Player Balance after Closure: ",
weth.balanceOf(testPlayers[0])
);
console.log(
"Expected Player Balance after Closure: ",
playerBalanceBeforeClosure +
((potBalance - (potBalance / 10)) / numberOfClaimers)
);
console.log("Pot Balance after Closure: ", weth.balanceOf(testPot));
console.log("Expected Pot Balance after Closure: 0");
​
assertEq(
weth.balanceOf(testPlayers[0]),
playerBalanceBeforeClosure +
((potBalance - (potBalance / 10)) / numberOfClaimers)
);
assertEq(weth.balanceOf(testPot), 0);
}

Finally run the tests with:

forge test

Recommended Mitigation

Replace i_players.length with claimants.length in the calculation of claimantCut within the Pot::closePot function to ensure the remaining rewards are correctly distributed among the actual claimants.

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;
+ uint256 claimantCut = (remainingRewards - managerCut) /
+ claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect distribution in closePot()

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.