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

Incorrect Prize Amount Emitted in GameEnded Event Due to Premature State Reset

devansh2020raulo

Root + Impact

The declareWinner() function contains a state management error where it resets the pot variable to 0 before emitting the GameEnded event, causing the event to emit an incorrect prize amount of 0 instead of the actual prize won by the winner.

Description

  • The normal behavior should emit the actual prize amount that the winner receives in the GameEnded event

  • The specific issue is in lines 240-243 of src/Game.sol where the pot is reset before the event emission

// Current buggy code in src/Game.sol lines 240-243
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
​
emit GameEnded(currentKing, pot, block.timestamp, gameRound); // Emits 0 instead of actual prize

Risk

Likelihood:

  • This occurs every time declareWinner() is called (100% occurrence rate)

  • The function will be called regularly as part of normal game flow

Impact:

  • Off-chain applications and frontends will receive incorrect prize amount information

  • Game analytics and historical tracking will show all games ended with 0 prize amounts

  • Users and interfaces cannot properly track actual winnings from events

Proof of Concept

This test demonstrates how the declareWinner() function emits incorrect prize amount data in the GameEnded event. The test sets up a scenario where a player claims the throne, the grace period expires, and then declareWinner() is called. We expect the event to emit the actual prize amount that was accumulated in the pot, but due to the premature reset of the pot variable, the event emits 0 instead.

function testIncorrectPrizeAmountInEvent() public {
// Setup: Player1 claims throne and grace period expires
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
uint256 expectedPrize = game.pot();
// Fast forward past grace period
vm.warp(block.timestamp + GRACE_PERIOD + 1);
// Expect event to emit the actual prize amount, but it will emit 0
vm.expectEmit(true, false, false, true);
emit GameEnded(player1, expectedPrize, block.timestamp, 1); // This will fail
game.declareWinner(); // Event will actually emit 0 for prize amount
}

Recommended Mitigation

The fix involves storing the pot value in a temporary variable before resetting it to zero. This ensures that when the GameEnded event is emitted, it contains the actual prize amount that the winner received, rather than the reset value of 0. This maintains data integrity for off-chain applications and provides accurate historical records.

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;
​
+ uint256 prizeAmount = pot; // Store prize amount before resetting
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
​
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, prizeAmount, 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.