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

Incorrect Prize Disclosure in declareWinner()

usengo

Root + Impact

Description


  • When the grace period expires and a winner is declared, the declareWinner function should emit the GameEnded event with the correct prize amount representing the total pot awarded to the winner. This allows players and off-chain services to accurately track and verify the prize.

  • Currently, the declareWinner function sets the pot to zero before emitting the GameEnded event, causing the event to always report a prize amount of zero regardless of the actual winnings. This leads to incorrect and misleading event data, reducing transparency and potentially undermining player trust.

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;
​
// Root cause: pot is zeroed BEFORE emitting the event,
// so event shows prizeAmount as 0 instead of the actual pot value
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // @> Pot is reset here too early
​
// @> The event uses the pot AFTER it has been reset to zero
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}
​

Risk

Likelihood:

  • This issue occurs every time the declareWinner() function is called after the grace period expires, as the contract always emits the GameEnded event immediately after resetting the pot to zero.

  • Since the event emission is part of the normal game flow to announce the winner, the bug will manifest consistently in all rounds where a winner is declared, making it very likely in regular usage.

Impact:

  • Off-chain services, explorers, and players relying on event logs will see the prize amount as zero, causing confusion and loss of transparency about the actual winnings.

  • This undermines trust in the game’s fairness and transparency, potentially damaging user confidence and discouraging participation, even though no funds are actually lost or stolen.

Proof of Concept

Explanation

The test simulates a player claiming the throne, waits until after the grace period, and calls declareWinner(). The GameEnded event incorrectly emits prizeAmount = 0 because pot is reset before the event is emitted, causing event data to not match actual winnings.

​
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import "forge-std/Test.sol";
import "../src/Game.sol";
​
/**
* @title GameEventBugTest
* @notice Proof of Concept for incorrect prizeAmount emission in GameEnded event.
* Works both before and after the fix by toggling the `expectBug` flag.
*/
contract GameEventBugTest is Test {
Game game;
​
// Copy the event from Game so we can use `expectEmit`
event GameEnded(
address indexed winner,
uint256 prizeAmount,
uint256 timestamp,
uint256 round
);
​
// Toggle this to `true` if testing buggy version, `false` for fixed version
bool expectBug = false;
​
function setUp() public {
// Deploy Game with parameters: claimFee=1 ETH, gracePeriod=60s, feeIncrease=10%, platformFee=5%
game = new Game(1 ether, 60, 10, 5);
}
​
function test_DeclareWinner_PrizeAmountEmission() public {
vm.deal(address(1), 5 ether);
​
// Step 1: Player1 claims the throne
vm.startPrank(address(1));
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
​
// Step 2: Simulate grace period expiry
vm.warp(block.timestamp + game.gracePeriod() + 1);
​
// Step 3: Expect correct or buggy prize emission
vm.expectEmit(true, true, false, true);
​
if (expectBug) {
// Buggy behavior: pot is reset before emit, so prize = 0
emit GameEnded(address(1), 0, block.timestamp, 1);
} else {
// Fixed behavior: emit actual pot amount
uint256 prize = game.pot();
emit GameEnded(address(1), prize, block.timestamp, 1);
}
​
// Trigger the emission
game.declareWinner();
}
}
​
​

Recommended Mitigation

Explanation

Save the current pot into a local variable before resetting it, and use that variable when emitting the GameEnded event. This ensures the event reflects the true prize amount.

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