Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

_isValidPercantage modifier could lead to reward deduction.

nkh000

Modifier isValidPercantage does not check how much is the percantage value is to be set but only checks if the given number is between 0 and 100.

Description

When a value is passed inside the modifier, it only checks to see if the given number is between the range of 0 and 100 which is correct for a percantage value but this could lead to any number between 0 to 100 which could cause complete deduction in rewards . Even if the owner is trusted person, this could happen by mistake and can lead to 0 rewards in some cases.

  • Generally, for any place in the contract where a percantage value is needed it would work efficiently.

  • However, if 100 is passed in for 100% of deduction in the amount, this could lead to zero rewards for users making protocol untrust worthy.

modifier isValidPercentage(uint256 _percentage) {
@> require(_percentage <= 100, "Game: Percentage must be 0-100.");
_;
}
​
// which if is set to 100 than in cases like claimThrone function
@> currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
// it could lead 100% of the sentAmount to become the platform fee, leaving nothing for the pot

Risk

Likelihood: LOW

  • Only happens when the owner of the contract does this and owner is trusted in most of the cases.

  • But could be very dangerous if the owener is not a trusted person.

Impact: HIGH/MEDIUM

  • Can lead to loss of rewards for the user

  • Creating a very bad impression of the Game to the user.

Proof of Concept

In the test function below, we can see that if owners set the platformPercantageFee which is later validated using isValidPercantage modifier, this causes protocol take the entire amount as the platform fee and adding no amount to winning pot. Similar for all the other functions where isValidPercantage is used .

function testInvalidPercantageValidation() public {
vm.prank(deployer);
game.updatePlatformFeePercentage(100);
address player = makeAddr("player");
vm.deal(player, 1 ether);
vm.prank(player);
game.claimThrone{value: 1 ether}();
​
vm.warp(block.timestamp + GRACE_PERIOD + 1 seconds);
game.declareWinner();
​
vm.prank(player);
vm.expectRevert("Game: No winnings to withdraw.");
game.withdrawWinnings();
}

Recommended Mitigation

With this approach, In different functions like updatePlatformFeePercentage and updateClaimFeeParameters we can set different and realistic percantage value within the minimum or maximum range.

- modifier isValidPercentage(uint256 _percentage) {
- require(_percentage <= 100, "Game: Percentage must be 0-100.");
​
+ modifier isValidPercentage(uint256 _percentage, uint256 _min, uint256 _max) {
+ require(_max <= 100, "Max percentage cannot exceed 100.");
+ require(_percentage >= _min && _percentage <= _max, "Percentage must be within valid range of minimum and maximum value.");
_;
}
​
​
Updates

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!