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

declareWinner() emits incorrect pot value (always 0).

0xdelvine

Root + Impact

Description

The declareWinner function is designed to finalize the game by marking it as ended, assigning the current pot to the pendingWinnings of the currentKing, and emitting a GameEnded event to log the winner and prize amount. However, the pot is reset to 0 before emitting the GameEnded event, causing the event to incorrectly log a prizeAmount of 0 instead of the actual pot value.

// 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; // Reset pot before emitting event
@> emit GameEnded(currentKing, pot, block.timestamp, gameRound);

}

Risk

Likelihood :

  • Occurs every time the declareWinner function is called after the grace period expires, as the pot is always reset before the GameEnded event is emitted.

  • Affects all game rounds where a winner is declared, since the event emission logic is part of the function’s core execution path.

Impact :

  • Misleads users and frontend applications by logging a prizeAmount of 0 in the GameEnded event, despite the actual pot being transferred to the winner’s pendingWinnings.

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.20;
​
import "forge-std/Test.sol";
import "../src/Game.sol"; // Assume your contract is named Game.sol
​
contract GameEventPoCTest is Test {
Game game;
address alice = vm.addr(1);
​
function setUp() public {
// Deploy the game with reasonable parameters
game = new Game(
1 ether, // initialClaimFee
1 days, // gracePeriod
10, // feeIncreasePercentage
5 // platformFeePercentage
);
​
// Fund Alice
vm.deal(alice, 10 ether);
}
​
function testGameEndedEventLogsZeroPrize() public {
// Fix: currentKing is in slot 1, not 0 (due to Ownable)
vm.store(
address(game),
bytes32(uint256(1)), // slot 1 holds currentKing
bytes32(uint256(uint160(alice)))
);
​
// Fund Alice and let her call claimThrone (must match msg.sender == currentKing due to bug)
vm.deal(alice, 10 ether);
vm.prank(alice);
game.claimThrone{value: 1 ether}(); // This sets lastClaimTime
​
// Wait for grace period to pass
vm.warp(block.timestamp + 2 days);
​
// Expect broken GameEnded event
vm.expectEmit(true, true, true, true);
emit Game.GameEnded(alice, 0, block.timestamp, 1);
​
// Trigger game end
game.declareWinner();
​
// Check actual winnings assigned
assertGt(game.pendingWinnings(alice), 0, "Alice should have real winnings despite zero prize emitted");
}
}```
​
In the test's expectation:
​
```solidity
emit Game.GameEnded(alice, 0, block.timestamp, 1);

  • The contract has a bug that causes the emitted prize to always be 0

  • So, to confirm the buggy behavior, we explicitly expect the event to log 0

  • At the same time, we assert that the real reward was added to the winner’s balance:

assertGt(game.pendingWinnings(alice), 0);

This proves that the winner got paid internally, but the event tells a misleading story — which can break off-chain indexing, analytics, or user UIs relying on the event logs.

This proof illustrates a mismatch between the internal contract state and the publicly emitted event data. While the correct balance is assigned internally, the external-facing event tells a different and incorrect story.

Recommended Mitigation

- pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
- pot = 0;
-
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ uint256 prizeAmount = pot;
+ pendingWinnings[currentKing] = pendingWinnings[currentKing] + prizeAmount;
+ pot = 0;
+
+ emit GameEnded(currentKing, prizeAmount, block.timestamp, gameRound);

This change ensures that the GameEnded event accurately reflects the prize awarded at the time of emission. Capturing the pot value in a local variable before resetting it allows the event to log correct and reliable data.

Updates

Appeal created

inallhonesty Lead Judge about 2 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.