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

Period `Game::claimThrone` Ended but still can use `Game::claimThrone`, make the protocol unreliable

vegazeronos

Period Game::claimThrone Ended but still can use Game::claimThrone, make the protocol unreliable

Description

  • In readme.md said Cannot claim if the game has ended. But if the period of game already ended, it should be finish and no one should be able to claimThrone anymore. This make the protocol limit the time of the game so the game can be finished.

  • As shown on the code below, there is no checking the time bound to this period of game

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;

Risk

Likelihood:

  • This will occur very likely , because anyone can do it.

Impact:

  • This can make the protocol break

Proof of Concept

  1. The period of game has already finished

  2. There's player who want to jjoin the game and use th Game::claimThrone()

  3. The period of game is changing and the game started again

Please place this code on the Game.t.sol.

function test_canClaimThroneEvenGracePeriodGameHasEnded() public {
uint256 expectedFinishGame = game.gracePeriod() + game.lastClaimTime();
vm.warp(100 days);
vm.startPrank(player1);
// vm.expectRevert();
game.claimThrone{value: 1 ether}();
uint256 actualFinishGame = block.timestamp;
vm.stopPrank();
​
//player still can claimThrone() even after the graceperiod finish...
assertGt(actualFinishGame, expectedFinishGame);
//actualFinishGame = 8640000 Sat Apr 11 1970 07:00:00
//expectedFinishGame = 259201 Sun Jan 04 1970 07:00:01
}

Recommended Mitigation

Please add the require statement to check if the period of the game has been expired or not.

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(block.timestamp < lastClaimTime + gracePeriod, "Game: Grace period has been expired.");
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
.
.
.
}
​
Updates

Appeal created

inallhonesty Lead Judge 27 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::claimThrone can still be called regardless of the grace period

Support

FAQs

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