Submission Details
Severity:
Inverted logic in `claimThrone()` makes the entire game unusable.
Root + Impact
Description
The require statement:"require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.")"; in `claimThrone()` has inverted logic, preventing anyone except `address(0)` from claiming the throne, which renders the entire game unusable.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
@> require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
// Calculate platform fee
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
// Defensive check to ensure platformFee doesn't exceed available amount after previousKingPayout
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
// Remaining amount goes to the pot
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;
// Update game state
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
// Increase the claim fee for the next player
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(msg.sender, sentAmount, claimFee, pot, block.timestamp);
}
Risk
Likelihood:
-
When any real player tries joining the game.
Impact:
-
No one can participate in the game, rendering the protocol unusable.
Proof of Concept
1. The game is deployed and the current king is address(0) which I verify with my assert statement.
2. I hoax as address(0) and call the claimThrone function which goes through
3. I assert that address(0) is still the king.
4. I prank as player 1, call claimThrone. The function reverts showing this error:
[Revert] Game: You are already the king. No need to re-claim.
function test_AddressZeroCanCallClaimThroneAgainAndOtherPlayersCannotJoin() public {
// Assert that the currentKing is address(0) at deployment
assert(game.currentKing() == address(0));
// Store the initial king address for comparison
address currentKingAtInitalDeployment = game.currentKing();
// Give address(0) 10 ether and set it as msg.sender
hoax(address(0), 10 ether);
// Address(0) successfully claims the throne (this shouldn't be possible)
game.claimThrone{value: INITIAL_CLAIM_FEE}();
// Store the king address after the claim
address currentKingPostClaimThrone = game.currentKing();
// Verify that address(0) is still the king (proving no state change)
assert(currentKingAtInitalDeployment == currentKingPostClaimThrone);
// Switch to player1 as msg.sender
vm.prank(player1);
// Get the current claim fee (which has increased after the previous claim)
uint256 currentClaimFee = game.claimFee();
// Expect the transaction to revert when a real player tries to claim
vm.expectRevert();
// Player1 attempts to claim throne but gets reverted due to inverted logic
game.claimThrone{value: currentClaimFee}();
}
Traces:
[185157] GameTest::test_AddressZeroCanCallClaimThroneAgainAndOtherPlayersCannotJoin()
├─ [2618] Game::currentKing() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ [618] Game::currentKing() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ [0] VM::deal(0x0000000000000000000000000000000000000000, 10000000000000000000 [1e19])
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [130721] Game::claimThrone{value: 100000000000000000}()
│ ├─ emit ThroneClaimed(newKing: 0x0000000000000000000000000000000000000000, claimAmount: 100000000000000000 [1e17], newClaimFee: 110000000000000000 [1.1e17], newPot: 95000000000000000 [9.5e16], timestamp: 1)
│ └─ ← [Stop]
├─ [618] Game::currentKing() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ [0] VM::prank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [514] Game::claimFee() [staticcall]
│ └─ ← [Return] 110000000000000000 [1.1e17]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [21259] Game::claimThrone{value: 110000000000000000}()
│ └─ ← [Revert] Game: You are already the king. No need to re-claim.
└─ ← [Stop]
Recommended Mitigation
- require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
+ require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
This mitigation works because the require statement now checks that the msg.sender is NOT the current king. Fixing the functionality of the protocol.
H
Updates
Appeal created
inallhonesty about 1 month 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.