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

Incorrect equality check in claimThrone() function causes global denial-of-service--No new players can claim the throne

phylax

Description

  • When calling the Game::claimThrone function players send a fee of value claimFee to become the new king. The function should check if the msg.sender is not the king and if it is then revert.

  • In the function the require statement has the wrong equality check. It checks is the msg.sender is the king and reverts if not. This blocks anybody from claiming the throne.

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.");
// rest of logic
​

Risk

Likelihood:

  • The likelihood for this to happen is high because it blocks any player to claim the throne.

Impact:

  • Causing a denial of service blocking any players to claim the throne.

Proof of Concept

Add this test to the suite:

function testClaimThrone() public {
vm.prank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
}

Will get an output like this:

Ran 1 test for test/Game.t.sol:GameTest
[PASS] testClaimThrone() (gas: 46982)
Traces:
[46982] GameTest::testClaimThrone()
├─ [0] VM::prank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf28dceb3: 4Game: You are already the king. No need to re-claim.)
│ └─ ← [Return]
├─ [29259] Game::claimThrone{value: 100000000000000000}()
│ └─ ← [Revert] Game: You are already the king. No need to re-claim.
└─ ← [Stop]

Recommended Mitigation

Instead of having the require statement check if the king is the msg.sender instead check is the king is NOT the msg.sender.

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.");
// rest of logic
​
Updates

Appeal created

inallhonesty Lead Judge 4 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.

Give us feedback!