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

Incorrect Game End Emit Value

mavrikdev

Root + Impact

Description

  • When a game is successfully ended via declareWinner, the details of the game will be emitted for public record.

  • The declareWinner function mistakenly resets the pot storage value to zero before referring to that same value in the emit message. This means every game will emit that the pot was empty.

// Root cause in the codebase with @> marks to highlight the relevant section
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; //<----- root cause
​
emit GameEnded(currentKing, pot, block.timestamp, gameRound); // <----- root cause
}

Risk

Likelihood:

  • This occurs everytime a game winner is declared.

Impact:

  • Low impact: This will affect any data collection services relying on these emit messages such as TheGraph

Proof of Concept

This test simulates a game round with player1 winning. When the player triggers declareWinner it does emit with a 0 value for the pot.

function test_IncorrectEmitValue() public {
vm.prank(deployer);
game = new Game(INITIAL_CLAIM_FEE, GRACE_PERIOD, FEE_INCREASE_PERCENTAGE, PLATFORM_FEE_PERCENTAGE);
​
vm.startPrank(player1);
game.claimThrone{value: .15 ether}();
​
vm.warp(block.timestamp + 2 days);
​
uint256 now = block.timestamp;
​
vm.expectEmit();
emit Game.GameEnded(player1, 0, now, game.gameRound()); // Declare winner will always emit empty pot
game.declareWinner();
}

Recommended Mitigation

Make a local reference to the pot storage variable to maintain the correct value after zero reseting the storage value.

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