Missing Access Control on Critical Parameter Updates
Description
-
The contract allows the owner to update critical game parameters like grace period, claim fee parameters and platform fee percentage during active gameplay
-
These parameter changes can be made at any time during a game round, potentially manipulating ongoing games to benefit the owner or harm current players
function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner {
require(_newGracePeriod > 0, "Game: New grace period must be greater than zero.");
gracePeriod = _newGracePeriod;
emit GracePeriodUpdated(_newGracePeriod);
}
function updatePlatformFeePercentage(uint256 _newPlatformFeePercentage)
external
onlyOwner
isValidPercentage(_newPlatformFeePercentage)
{
platformFeePercentage = _newPlatformFeePercentage;
emit PlatformFeePercentageUpdated(_newPlatformFeePercentage);
}
function updateClaimFeeParameters(
uint256 _newInitialClaimFee,
uint256 _newFeeIncreasePercentage
) external onlyOwner isValidPercentage(_newFeeIncreasePercentage) {
initialClaimFee = _newInitialClaimFee;
feeIncreasePercentage = _newFeeIncreasePercentage;
emit ClaimFeeParametersUpdated(_newInitialClaimFee, _newFeeIncreasePercentage);
}
Risk
Likelihood: Medium
-
Owner has incentive to manipulate parameters for profit
-
Changes can be made at any time without player consent
-
No restrictions prevent mid-game manipulation
Impact: Medium
-
Owner can reduce grace period to rush game endings when beneficial
-
Platform fees can be increased to extract more revenue from ongoing claims
-
Initial claim fee and fee increase percentage can be manipulated mid-game
-
Players lose trust in fair gameplay
-
Game parameters become unpredictable during active rounds
Proof of Concept
The tests demonstrate how each parameter update function can manipulate active gameplay:
function testUpdateGracePeriod_CanManipulateActiveGame() public {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.prank(deployer);
game.updateGracePeriod(1 hours);
assertEq(game.gracePeriod(), 1 hours);
}
function testUpdatePlatformFeePercentage_CanManipulateActiveGame() public {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.prank(deployer);
game.updatePlatformFeePercentage(50);
assertEq(game.platformFeePercentage(), 50);
}
function testUpdateClaimFeeParameters_CanManipulateActiveGame() public {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.prank(deployer);
game.updateClaimFeeParameters(1 ether, 50);
assertEq(game.initialClaimFee(), 1 ether);
assertEq(game.feeIncreasePercentage(), 50);
}
Explanation: Each test shows how the owner can manipulate different critical game parameters during active gameplay, potentially affecting fair competition and game economics.
Recommended Mitigation
- function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner {
+ function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner gameEndedOnly {
require(_newGracePeriod > 0, "Game: New grace period must be greater than zero.");
gracePeriod = _newGracePeriod;
emit GracePeriodUpdated(_newGracePeriod);
}
function updatePlatformFeePercentage(uint256 _newPlatformFeePercentage)
external
onlyOwner
+ gameEndedOnly
isValidPercentage(_newPlatformFeePercentage)
{
platformFeePercentage = _newPlatformFeePercentage;
emit PlatformFeePercentageUpdated(_newPlatformFeePercentage);
}
+ function updateClaimFeeParameters(
+ uint256 _newInitialClaimFee,
+ uint256 _newFeeIncreasePercentage
+ ) external onlyOwner gameEndedOnly isValidPercentage(_newFeeIncreasePercentage) {
require(_newInitialClaimFee > 0, "Game: New initial claim fee must be greater than zero.");
initialClaimFee = _newInitialClaimFee;
feeIncreasePercentage = _newFeeIncreasePercentage;
emit ClaimFeeParametersUpdated(_newInitialClaimFee, _newFeeIncreasePercentage);
}
Explanation: Use the existing gameEndedOnly modifier to restrict parameter updates to only occur when games have ended, preventing mid-game manipulation and ensuring fair gameplay.
