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

`GameEnded` Logs Zero Prize results in Users & Indexers See 0 ETH Won

rootkit677

GameEnded Logs Zero Prize (Users & Indexers See 0 ETH Won)

Description

  • Expected behaviour: When declareWinner() is called after the grace period, the GameEnded event should emit the actual prize amount the winner is entitled to withdraw so that UIs, analytics dashboards and indexers can display accurate information.

  • Actual issue: The contract writes the winner’s pot to pendingWinnings and then sets pot = 0 before emitting the GameEnded event. Because the event arguments reference pot, the logged prizeAmount is always zero regardless of the real winnings.

// Game.sol — declareWinner()
@> 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 real prize

Risk

Likelihood:

  • declareWinner() will be called at the end of every successful game round, so the buggy event is emitted routinely.

  • Off-chain services (The Graph, Dune, custom front-ends) rely on events rather than storage reads, so they will almost always consume the incorrect value.

Impact:

  • Users and explorers display a prize of 0 ETH, misleading players and external observers.

  • Automated tooling that uses the event data for accounting or rewards distribution can malfunction or under-credit users.

Proof of Concept

The PoC below simulates a full game cycle: a player becomes king, the grace period elapses, and declareWinner() is called. We record logs and assert that the prizeAmount indexed in GameEnded is 0 even though the true winnings are non-zero.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
import "forge-std/Vm.sol";
​
/// @title PoC for GameEnded event logging 0 prize instead of real amount
/// @dev Demonstrates that GameEnded emits prizeAmount = 0 regardless of pot
contract PoCZeroPrizeEvent is Test {
Game internal game;
address internal player = address(1);
​
// Convenient constants for deployment
uint256 internal constant INITIAL_CLAIM_FEE = 0.1 ether;
uint256 internal constant GRACE_PERIOD = 1 days;
uint256 internal constant FEE_INCREASE_PERCENTAGE = 10;
uint256 internal constant PLATFORM_FEE_PERCENTAGE = 5;
​
function setUp() public {
// Fund player and deploy game
vm.deal(player, 1 ether);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
​
// player claims throne to build pot
vm.prank(player);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
}
​
/// @notice Verifies that GameEnded event logs zero prize even though winnings exist
function test_GameEnded_EmitsZeroPrize() public {
// Fast-forward beyond grace period
skip(GRACE_PERIOD + 1);
// Start recording logs
vm.recordLogs();
game.declareWinner();
// Obtain logs and decode first event
Vm.Log[] memory entries = vm.getRecordedLogs();
require(entries.length > 0, "No logs recorded");
​
// Decode ABI-encoded data: (uint256 prizeAmount, uint256 timestamp, uint256 round)
(uint256 loggedPrize,,) = abi.decode(entries[0].data, (uint256, uint256, uint256));
​
uint256 truePrize = game.pendingWinnings(player);
assertGt(truePrize, 0, "True prize should be positive");
assertEq(loggedPrize, 0, "Logged prize incorrectly zero");
}
}

Recommended Mitigation

Emit the event before zeroing the pot or cache the pot value in a local variable and pass it to the event. The one-line diff below adopts the second approach, which avoids re-ordering storage writes and keeps semantics explicit.

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);
+
+ uint256 prize = pot; // cache before zeroing
+ pendingWinnings[currentKing] += prize;
+ pot = 0;
+ emit GameEnded(currentKing, prize, block.timestamp, gameRound);
}

This change ensures indexers receive the correct prize amount while leaving storage layout and external behaviour intact.

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.