If platformFeePercentage is 100%, all user funds are taken as platform fees, and the pot does not increase.
Description
if the platformFeePercentage is a 100% all user funds are taken as platform fees and bypassing "Defensive check" because now currentPlatformFee == (sentAmount - previousKingPayout) and after that pot balance wil be 0 because its calculated from sentAmount - currentPlatformFee, but the vulnerability is not just that, the previousKingPayout variable doesn't initialize anywhere even after the previous king really payout because the variable it's only initialized inside the function, so since the previous king payout is initialized as 0 which means the "Defensive checks" is not gonna work because all user funds are taken as platform fees, and the pot does not increase.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
@> uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
@> if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}
Risk
Likelihood:
-
This vulnerability will occur when the platformFeePercentage is 100% or,
-
If the currentPlatformFee > (sentAmount - previousKingPayout) after the first calculation.
Impact:
Proof of Concept
here's the test:
function test_PotBecome0IfPlatformFeePercentageIs100() public {
uint256 platformFeePercentageBeforeUpdate = game.platformFeePercentage();
assertEq(platformFeePercentageBeforeUpdate, PLATFORM_FEE_PERCENTAGE);
console2.log("\n platform Fee Percentage Before Update: %s", platformFeePercentageBeforeUpdate,"%");
vm.startPrank(deployer);
game.updatePlatformFeePercentage(100);
vm.stopPrank();
uint256 platformFeePercentageAfterUpdate = game.platformFeePercentage();
assertEq(platformFeePercentageAfterUpdate, 100);
console2.log("\n platform Fee Percentage After Update: %s", platformFeePercentageAfterUpdate,"%");
uint256 platformFeesBalanceBefore = game.platformFeesBalance();
uint256 potBalanceBefore = game.pot();
assertEq(platformFeesBalanceBefore, 0);
assertEq(potBalanceBefore, 0);
console2.log("\n platform fees balance before user claim throne: %s", platformFeesBalanceBefore);
console2.log("\n pot balance before user claim throne: %s", potBalanceBefore);
vm.startPrank(player1);
game.claimThrone{value: 1 ether}();
vm.stopPrank();
uint256 platformFeesBalanceAfter = game.platformFeesBalance();
uint256 potBalanceAfter = game.pot();
assertEq(platformFeesBalanceAfter, 1 ether);
assertEq(potBalanceAfter, 0);
console2.log("\n platform fees balance after user claim throne: %s", platformFeesBalanceAfter);
console2.log("\n pot balance after user claim throne: %s", potBalanceAfter);
}
and the log from the test:
Ran 1 test for test/Game.t.sol:GameTest
[PASS] test_PotBecome0IfPlatformFeePercentageIs100() (gas: 160838)
Logs:
platform Fee Percentage Before Update: 5 %
platform Fee Percentage After Update: 100 %
platform fees balance before user claim throne: 0
pot balance before user claim throne: 0
platform fees balance after user claim throne: 1000000000000000000
pot balance after user claim throne: 0
Recommended Mitigation
Hard Limit on Platform Fee
Explicitly limit the platformFeePercentage value, Place in constructor, setter, or modifier:
+ require(platformFeePercentage <= 10, "Platform fee too high");
- if (currentPlatformFee > (sentAmount - previousKingPayout)) {
- currentPlatformFee = sentAmount - previousKingPayout;
- }
