Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Incorrect prerequite check on `msg.sender` with confusing revert message in `claimThrone` function

soupy

Summary

An incorrect prerequisite check on msg.sender in the claimThrone function causes the contract to become unusable after deployment. Since the currentKing is set as address(0) until first claim, attempt by any player to first call claimThrone reverts with the confusing message "Game: You are already the king. No need to re-claim." As a result, no other players can join or claim the throne, rendering the game non-functional from the start.

Description

In claimThrone function, the prerequisite check on msg.sender to be the currentKing resulting the contract is unusable after deployment as the currentKing is set as address(0) as start.

When any player with non-zero address to first call claimThrone, it will fail the check at require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");

currentKing is address(0)

contract Game is Ownable {
​
// --- State Variables ---
​
// Game Core State
<@@> address public currentKing; // The address of the current "King"
uint256 public lastClaimTime; // Timestamp when the throne was last claimed
...
​
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.");
....
​
gameEnded = false;
<@@> // currentKing starts as address(0) until first claim
}
​
}

Wrong prerequisite check on msg.sender == currentKing:

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.");
​
....
}

Risk

Likelihood:

  • Unusable right after contract was deployed as no player can successfully call the claimThrone function to participate the game

Impact:

  • Contract is unusable after deployment with no players, no funding streaming in

Proof of Concept

In test/Game.t.sol, add the following test:

function test_audit_incorrectPrequisiteCheckInClaimThrone() public {
console2.log("currentKing: ", game.currentKing());
​
// attempt by player 1
vm.prank(player1);
console2.log("msg.sender1: ", player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
​
// attempt by player 2
vm.prank(player2);
console2.log("msg.sender2: ", player2);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
​
// attempt by player 3
vm.prank(player3);
console2.log("msg.sender3: ", player3);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
}

In terminal, run forge test --match-test test_audit_incorrectPrequisiteCheckInClaimThrone -vvv will generate the following results:

$ forge test --match-test test_audit_incorrectPrequisiteCheckInClaimThrone -vvv [18:27:45]
[⠊] Compiling...
[⠔] Compiling 1 files with Solc 0.8.28
[⠒] Solc 0.8.28 finished in 460.41ms
Compiler run successful!
​
Ran 1 test for test/Game.t.sol:GameTest
[PASS] test_audit_incorrectPrequisiteCheckInClaimThrone() (gas: 130627)
Logs:
currentKing: 0x0000000000000000000000000000000000000000
msg.sender1: 0x7026B763CBE7d4E72049EA67E89326432a50ef84
msg.sender2: 0xEb0A3b7B96C1883858292F0039161abD287E3324
msg.sender3: 0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 738.21µs (97.33µs CPU time)
​
Ran 1 test suite in 117.12ms (738.21µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

The test passed with revert message "Game: You are already the king. No need to re-claim." for all different players who attempted to first call the claimThrone function, indicating that no player can successfully claim throne after the contract was deployed.

Recommended Mitigation

To correct the prerequisite condition check on msg.sender, so it is functional as intented and tally to revert message shall it fail

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.");
+ require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
​
uint256 sentAmount = msg.value;
....
}
Updates

Appeal created

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::claimThrone `msg.sender == currentKing` check is busted

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.