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

[L-3] The `Game::declareWinner` function emits the `Game::GameEnded` with incorrect `pot` value.

thecil

Root + Impact

[L-3] The Game::declareWinner function emits the Game::GameEnded with incorrect pot value.

Description

The Game::declareWinner function is used to declare a winner of the actual round once the grace period has passed and emit the Game::GameEnded event.

function declareWinner() external gameNotEnded {
require(currentKing != address(0), "Game: No one has claimed the throne yet.");
require(
block.timestamp > lastClaimTime + gracePeriod,
"Game: Grace period has not expired yet."
);
​
gameEnded = true;
​
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
​
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}

The pot global value is reset to zero before emitting the Game::GameEnded event, making the pot value equals to zero when the event is emitted.

Risk

Likelihood: Low

  • Reason 1 : The pot global value is reset to zero before emitting the Game::GameEnded event

Impact: Low

  • Impact 1: the Game::GameEnded event, triggers with the pot value equals to zero when the event is emitted.

Proof of Concept

The Game::declareWinner function emits the Game::GameEnded event with an incorrect pot value.

The pot global value is reset to zero before emitting the Game::GameEnded event, making the pot value equals to zero when the event is emitted.

The following unit test demonstrate the game round functionality with a winner:

function _claimThroneByUser(address _player, uint256 _fee) internal {
vm.startPrank(_player);
game.claimThrone{value: _fee}();
vm.stopPrank();
}
​
function test_game_round_with_winner() public {
// claim throne as player 1
_claimThroneByUser(player1, INITIAL_CLAIM_FEE);
​
vm.warp(block.timestamp + 1 hours);
​
uint256 expectedNewFee = game.claimFee() +
(game.claimFee() * FEE_INCREASE_PERCENTAGE) /
100;
// claim throne as player 2
_claimThroneByUser(player2, game.claimFee());
assertEq(game.claimFee(), expectedNewFee);
​
// will revert if grace period not reached
vm.expectRevert("Game: Grace period has not expired yet.");
game.declareWinner();
// increase time to finish the game round
vm.warp(
block.timestamp + game.getRemainingTime() + game.lastClaimTime()
);
// declare winner
game.declareWinner();
​
uint256 pendingWinAmount = game.pendingWinnings(player2);
uint256 player2BalanceBeforeWithdraw = player2.balance;
// withdraw winnings after declaring winner
vm.startPrank(player2);
game.withdrawWinnings();
vm.stopPrank();
assertEq(
player2.balance,
player2BalanceBeforeWithdraw + pendingWinAmount,
"Winner should receive the winnings."
);
assertEq(game.gameEnded(), true);
}

Executing the test with verbosity level 4, will show the emitted events during its execution and we can verify the incorrect pot value on the Game::GameEnded.

forge test --mt test_game_round_with_winner -vvvv

The test output will show the emitted event like this:

+- [48801] Game::declareWinner()
+---- emit GameEnded(winner: player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324], prizeAmount: 0, timestamp: 93602 [9.36e4], round: 1)

Recommended Mitigation

Snapshot the pot value before resetting it and pass it to the emitted event.

function declareWinner() external gameNotEnded {
require(
currentKing != address(0),
"Game: No one has claimed the throne yet."
);
require(
block.timestamp > lastClaimTime + gracePeriod,
"Game: Grace period has not expired yet."
);
​
gameEnded = true;
​
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
+ uint256 actualPot = pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, actualPot, block.timestamp, gameRound);
}
Updates

Appeal created

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::declareWinner emits GameEnded event with pot = 0 always

Support

FAQs

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