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

`GameEnded` Event Emits Zero Value

superdevfavour

GameEnded Event Emits Zero Value for Pot Prize when winner is declared, Providing Misleading Data to Off-Chain Services

Description

  • Normal Behavior: When a winner is declared, the GameEnded event should be emitted with the correct winner address and the prizeAmount they have won. This allows off-chain services, front-ends, and analytics platforms to accurately track game history.

  • The Issue: In the declareWinner function, the state variable pot is set to 0 before the GameEnded event is emitted. The event is then called using the now-zeroed pot variable as the prizeAmount parameter. As a result, every GameEnded event will report a prize of 0, which is incorrect and misleading.

/**
* @dev Allows anyone to declare a winner if the grace period has expired.
* The currentKing at the time the grace period expires becomes the winner.
* The pot is then made available for the winner to withdraw.
*/
function declareWinner() external gameNotEnded {
require(
currentKing != address(0),
"Game: No one has claimed the throne yet."
);
require(
block.timestamp > lastClaimTime + gracePeriod, //@audit check for grace period
"Game: Grace period has not expired yet."
);
​
gameEnded = true;
​
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot; //@audit check if the pot is added to in the claim fee
//@> pot = 0; // Reset pot after assigning to winner's pending winnings
​
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}

Risk

Likelihood: HIGH

  • This will happen for every game round that concludes with a declared winner.

Impact: LOW

  • This bug does not cause a loss of funds within the contract, as the prize is correctly assigned to pendingWinnings. However, it corrupts the contract's data history as told by its events. Any application or user relying on these events to display game results or calculate statistics will be broken or show incorrect information, undermining the transparency of the game.

Proof of Concept

  • Play the game until the pot contains a non-zero amount (e.g., 5 ETH).

  • Let the grace period expire.

  • Call the declareWinner() function.

  • Observe: Use an explorer like Etherscan or a local script to inspect the logs for the GameEnded event. The winner parameter will be correct, but the prizeAmount parameter will be 0.

  • Expected Result: The prizeAmount parameter in the event log should be 5 ETH.

Add this code to the test file:

function testGameEndedEvent() public {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.startPrank(player2);
game.claimThrone{value: INITIAL_CLAIM_FEE * 2}();
vm.warp(block.timestamp + GRACE_PERIOD + 1);
vm.expectEmit();
emit Game.GameEnded(player2, 0, block.timestamp, 1);
game.declareWinner();
}

Recommended Mitigation

Store the prize amount in a local memory variable before zeroing out the pot state variable. Use this local variable in the event emission.

Modify the declareWinner function as follows:

function declareWinner() external gameNotEnded {
// ... (existing require checks)
​
gameEnded = true;
​
// Store the prize amount before zeroing the pot
+ uint256 prizeAmount = pot;
​
pendingWinnings[currentKing] = pendingWinnings[currentKing] + prizeAmount;
pot = 0; // Reset pot after assigning to winner's pending winnings
​
// Use the local variable for the event
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, prizeAmount, block.timestamp, gameRound);
}
Updates

Appeal created

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

Give us feedback!