Submission Details
Severity:
Denial of service, currentKing is stuck at address(0), no one can ever call claimThrone()
currentKing is stuck at address(0), no one can ever call claimThrone() causing denial of service
Description
claimThrone()will always revert becausecurrentKingis stuck with the value ofaddress(0).
Risk
Likelihood:
Reason 1: The constructor leaves
currentKingunitialized with the value ofaddress(0):
// Root cause in the codebase with@> marks to highlight the relevant section
constructor(
uint256 _initialClaimFee,
uint256 _gracePeriod,
uint256 _feeIncreasePercentage,
uint256 _platformFeePercentage
) Ownable(msg.sender) { // Set deployer as owner
require(_initialClaimFee > 0, "Game: Initial claim fee must be greater than zero.");
require(_gracePeriod > 0, "Game: Grace period must be greater than zero.");
require(_feeIncreasePercentage <= 100, "Game: Fee increase percentage must be 0-100.");
require(_platformFeePercentage <= 100, "Game: Platform fee percentage must be 0-100.");
initialClaimFee = _initialClaimFee;
initialGracePeriod = _gracePeriod;
feeIncreasePercentage = _feeIncreasePercentage;
platformFeePercentage = _platformFeePercentage;
// Initialize game state for the first round
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
lastClaimTime = block.timestamp; // Game starts immediately upon deployment
gameRound = 1;
gameEnded = false;
// currentKing starts as address(0) until first claim
}
Reason 2:
currentKingis set toaddress(0)inresetGame()as well:
function resetGame() external onlyOwner gameEndedOnly {
// @>
currentKing = address(0);
// @>
lastClaimTime = block.timestamp;
pot = 0;
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
// totalClaims is cumulative across rounds, not reset here, but could be if desired.
emit GameReset(gameRound, block.timestamp);
}
Impact:
Impact 1: This causes
claimThrone()to always revert on line 188 because the require check is incorrect:
require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
It should require that msg.sender != currentKing:
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
Proof of Concept
// SPDX-License-Identifier: MIT
import {Test, console2} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameTest is Test {
Game public game;
address public deployer;
address public player1;
address public player2;
address public player3;
address public maliciousActor;
// Initial game parameters for testing
uint256 public constant INITIAL_CLAIM_FEE = 0.1 ether; // 0.1 ETH
uint256 public constant GRACE_PERIOD = 1 days; // 1 day in seconds
uint256 public constant FEE_INCREASE_PERCENTAGE = 10; // 10%
uint256 public constant PLATFORM_FEE_PERCENTAGE = 5; // 5%
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
player2 = makeAddr("player2");
player3 = makeAddr("player3");
maliciousActor = makeAddr("maliciousActor");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.deal(player2, 10 ether);
vm.deal(player3, 10 ether);
vm.deal(maliciousActor, 10 ether);
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
function testConstructor_RevertInvalidGracePeriod() public {
vm.expectRevert("Game: Grace period must be greater than zero.");
new Game(INITIAL_CLAIM_FEE, 0, FEE_INCREASE_PERCENTAGE, PLATFORM_FEE_PERCENTAGE);
}
function test_claimThroneAlwaysReverts() public {
vm.startPrank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: 0.2 ether}();
vm.stopPrank();
}
}
Then run with:
forge test -vvv --match-test test_claimThroneAlwaysReverts
Sample output:
[⠊] Compiling...
[⠒] Compiling 1 files with Solc 0.8.28
[⠢] Solc 0.8.28 finished in 975.72ms
Compiler run successful!
Ran 1 test for test/Game.t.sol:GameTest
[PASS] test_claimThroneAlwaysReverts() (gas: 46785)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.18ms (92.58µs CPU time)
Ran 1 test suite in 9.81ms (1.18ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
claimThrone() on line 188 should require that msg.sender != currentKing:
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
Updates
Appeal created
inallhonesty about 2 months ago
Submission Judgement Published Assigned finding tags:
Game::claimThrone `msg.sender == currentKing` check is busted
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.