Summary
The contract owner can modify initial claim fee and fee increase percentage during an active game round, potentially disrupting game economics and player expectations.
Description
The updateClaimFeeParameters() function allows the owner to change initialClaimFee and feeIncreasePercentage at any time, including during active gameplay. While initialClaimFee only affects future rounds, feeIncreasePercentage immediately impacts how the current claimFee will grow in the ongoing game.
Root Cause
No restriction prevents fee parameter updates during active gameplay:
function updateClaimFeeParameters(
uint256 _newInitialClaimFee,
uint256 _newFeeIncreasePercentage
) external onlyOwner isValidPercentage(_newFeeIncreasePercentage) {
require(_newInitialClaimFee > 0, "Game: New initial claim fee must be greater than zero.");
initialClaimFee = _newInitialClaimFee;
feeIncreasePercentage = _newFeeIncreasePercentage;
emit ClaimFeeParametersUpdated(_newInitialClaimFee, _newFeeIncreasePercentage);
}
Impact
-
Economic Manipulation: Owner can change fee growth rate mid-game to favor certain outcomes
-
Unfair Advantage: Players who planned based on original fee increase rate are disadvantaged
-
Trust Issues: Game economics become unpredictable and manipulable
-
Strategic Disruption: Mid-game changes can invalidate player strategies
Proof of Concept
function testClaimFeeParametersUpdateMidGame() public {
uint256 originalFeeIncrease = game.feeIncreasePercentage();
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
uint256 expectedNextFee = INITIAL_CLAIM_FEE + (INITIAL_CLAIM_FEE * originalFeeIncrease) / 100;
assertEq(game.claimFee(), expectedNextFee);
vm.prank(deployer);
game.updateClaimFeeParameters(INITIAL_CLAIM_FEE, 50);
vm.prank(player2);
game.claimThrone{value: expectedNextFee}();
uint256 newExpectedFee = expectedNextFee + (expectedNextFee * 50) / 100;
assertEq(game.claimFee(), newExpectedFee);
assertGt(newExpectedFee, expectedNextFee * 140 / 100);
}
function testInitialClaimFeeUpdateImpact() public {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.warp(block.timestamp + GRACE_PERIOD + 1);
game.declareWinner();
vm.prank(deployer);
game.updateClaimFeeParameters(0.5 ether, FEE_INCREASE_PERCENTAGE);
vm.prank(deployer);
game.resetGame();
assertEq(game.claimFee(), 0.5 ether);
vm.prank(player2);
vm.expectRevert("Game: Insufficient ETH sent to claim the throne.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
}
Recommended Mitigation
Restrict fee parameter updates to prevent mid-game changes:
function updateClaimFeeParameters(
uint256 _newInitialClaimFee,
uint256 _newFeeIncreasePercentage
) external onlyOwner isValidPercentage(_newFeeIncreasePercentage) {
require(_newInitialClaimFee > 0, "Game: New initial claim fee must be greater than zero.");
require(gameEnded || currentKing == address(0), "Game: Cannot update fee parameters during active game");
initialClaimFee = _newInitialClaimFee;
feeIncreasePercentage = _newFeeIncreasePercentage;
emit ClaimFeeParametersUpdated(_newInitialClaimFee, _newFeeIncreasePercentage);
}
