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

Malicious Owner can change values of Game's core variables during a game which is not Ideal.

nkh000

While the Game is ongoing, it is not ideal to change the games core variables like percantage for fee calculation, gracePeriod and so on.

Description

Contract must not allow to change variables like feeIncreasePercantage , platformFeePercantage and gracePeriod during an ongoing game. This could lead user dissatisfaction. If a malicious Owner wants to steal user reward he/she could maliciously use this functions for thier advantage.

  • Once the game has begun, values must not be changed when round is complete.

  • Unneccessary, mistaken or malicious update to these values could lead user rewards loss.

@> function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner {
...
}
​
@> function updateClaimFeeParameters(
uint256 _newInitialClaimFee,
uint256 _newFeeIncreasePercentage
) external onlyOwner isValidPercentage(_newFeeIncreasePercentage) {
...
}
​
@> function updatePlatformFeePercentage(
uint256 _newPlatformFeePercentage
) external onlyOwner isValidPercentage(_newPlatformFeePercentage) {
...
}

Risk

Likelihood: LOW

  • Can only be called by the owner of the contract who is a trusted person/account.

  • But if the Owner is malicious than it could be very difficult for all the other participants to play a fair game.

Impact: HIGH

  • Malicious access to owner account or mistake update by the user could lead to errors like :

    • Grace Period becoming as low as 1 seconds making anyone or deployer itself to claim instantly after claiming the throne, fair participation would be impossible.

    • High platform fee percantage could take away most or all of the sent amount as a platform fee leading zero amount added to the pot and being added to the platformFee.

    • Sudden change in claimFee increase/decrease percantage could make participation harder for users.

Proof of Concept

In this test function, we Can clearly see how deployer could manipulate the variables and be able to earn more in rewards than the actual intended reward for a normal game round.

function testChangeOfCoreVariablesDuringTheGame() public {
vm.prank(player1);
game.claimThrone{value: 1 ether}();
​
vm.startPrank(player2);
game.claimThrone{value: 1 ether}();
vm.warp(block.timestamp + GRACE_PERIOD + 5 seconds);
game.declareWinner();
game.withdrawWinnings();
vm.stopPrank();
​
uint256 normalWiningBalanceOfUser = player2.balance;
​
vm.prank(deployer);
game.resetGame();
​
vm.prank(player3);
game.claimThrone{value: 1 ether}();
​
vm.startPrank(deployer);
game.updateGracePeriod(1 seconds);
game.updateClaimFeeParameters(0.1 ether, 0);
game.updatePlatformFeePercentage(1);
game.claimThrone{value: 1 ether}();
​
vm.warp(block.timestamp + 2 seconds);
game.declareWinner();
​
game.withdrawWinnings();
vm.stopPrank();
​
uint256 manipulatedWinningBalanceOfOwner = deployer.balance;
​
assert(manipulatedWinningBalanceOfOwner > normalWiningBalanceOfUser);
}

Recommended Mitigation

For this issue recommended mitigations are as follow :

  1. Do not allow changing of these important varaibles during the game by anyone. Use gameEndedOnly modifier in all of these functions to prevent this behavior.

  2. Do not let the Owner of the contract to play the game. Use a defensive check in claimThrone function line require(msg.sender != owener()) to improve more fairness in the game.

- function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner { ... }
+ function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner gameEndedOnly { ... }
​
- function updateClaimFeeParameters(uint256 _newInitialClaimFee, uint256 _newFeeIncreasePercentage ) external onlyOwner isValidPercentage(_newFeeIncreasePercentage) { ... }
+ function updateClaimFeeParameters(uint256 _newInitialClaimFee, uint256 _newFeeIncreasePercentage ) external onlyOwner gameEndedOnly isValidPercentage(_newFeeIncreasePercentage) { ... }
​
​
- function updatePlatformFeePercentage( uint256 _newPlatformFeePercentage) external onlyOwner isValidPercentage(_newPlatformFeePercentage) { ... }
+ function updatePlatformFeePercentage( uint256 _newPlatformFeePercentage) external onlyOwner gameEndedOnly isValidPercentage(_newPlatformFeePercentage) { ... }
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!