Centralized Control Allows Owner to Manipulate Live Game Dynamics
Description
The contract owner possesses unilateral and immediate power to change critical game parameters—gracePeriod, initialClaimFee, feeIncreasePercentage, and platformFeePercentage—at any time by calling the respective update functions (updateGracePeriod(), updateClaimFeeParameters(), etc.). These functions lack any access control modifier to prevent their execution during an active game round. This introduces a significant centralization risk, as a malicious or compromised owner can alter the game's rules mid-play to benefit themselves or specific 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 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);
}
@> function updatePlatformFeePercentage(uint256 _newPlatformFeePercentage)
external
onlyOwner
isValidPercentage(_newPlatformFeePercentage)
{
platformFeePercentage = _newPlatformFeePercentage;
emit PlatformFeePercentageUpdated(_newPlatformFeePercentage);
}
Risk
Likelihood:
-
The privileged functions can be executed in a single transaction by the owner at any moment. There are no technical barriers, such as a time-lock or a game state check, preventing their use during a live round.
Impact:
Proof of Concept
The ability for the owner to manipulate game outcomes can be demonstrated with a Foundry test. The following test function was added to the test/Game.t.sol file to simulate an owner ensuring their friend wins the game.
function testOwnerCanManipulateGracePeriodMidGame() public{
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
assertEq(game.currentKing(),player1,"Player 1 should be the king");
uint256 newClaimFee = game.claimFee();
vm.prank(player2);
game.claimThrone{value: newClaimFee}();
assertEq(game.currentKing(), player2, "Player 2 should now be the king");
assertFalse(game.gameEnded(),"Game should be still be active");
vm.prank(deployer);
uint256 maliciousGracePeriod = 1;
game.updateGracePeriod(maliciousGracePeriod);
assertEq(game.gracePeriod(),maliciousGracePeriod,"Grace period should be update to 1 second");
vm.warp(block.timestamp + 2);
vm.prank(deployer);
game.declareWinner();
assertTrue(game.gameEnded(),"Game should have ended");
uint256 platformFeeFromP1 = (INITIAL_CLAIM_FEE * PLATFORM_FEE_PERCENTAGE) / 100;
uint256 potAfterP1 = INITIAL_CLAIM_FEE - platformFeeFromP1;
uint256 platformFeeFromP2 = (newClaimFee * PLATFORM_FEE_PERCENTAGE) / 100;
uint256 potFromP2 = newClaimFee - platformFeeFromP2;
uint256 expectedPot = potAfterP1 + potFromP2;
assertEq(game.pendingWinnings(player2), expectedPot, "Player 2 should have winnings pending");
assertEq(game.pendingWinnings(player1), 0, "Player 1 should have no winnings");
}
Recommended Mitigation
Restrict parameter updates so they can only occur when a game round is not active. This can be achieved by adding the gameEndedOnly modifier to the parameter update functions. This ensures that changes can only be made after a winner is declared but before a new game starts.
- 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 updateClaimFeeParameters(
+ function updateClaimFeeParameters(
uint256 _newInitialClaimFee,
uint256 _newFeeIncreasePercentage
- ) external onlyOwner isValidPercentage(_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);
}
- function updatePlatformFeePercentage(uint256 _newPlatformFeePercentage)
- external
- onlyOwner
- isValidPercentage(_newPlatformFeePercentage)
+ function updatePlatformFeePercentage(uint256 _newPlatformFeePercentage) external onlyOwner gameEndedOnly isValidPercentage(_newPlatformFeePercentage)
{
platformFeePercentage = _newPlatformFeePercentage;
emit PlatformFeePercentageUpdated(_newPlatformFeePercentage);
}
