Root + Impact
Description
-
The GameEnded event should emit the actual prize amount that the winner receives when the game ends. According to the event documentation, the prizeAmount parameter represents "The total prize amount won." This should be the accumulated pot value that gets transferred to the winner's pending winnings.
-
The event incorrectly emits pot as the prize amount after it has already been reset to 0, instead of emitting the actual prize amount that was awarded to the winner
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;
@> emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}
Risk
Likelihood:
-
This bug occurs every single time declareWinner() is called, making it a 100% reproducible issue.
-
Every game that ends will emit incorrect prize information.
Impact:
-
This is an informational/logging issue that doesn't affect the core protocol functionality or put funds at risk
-
However, external systems, frontends, or analytics tools relying on this event data would receive incorrect information about prize amounts.
Proof of Concept
This test shows that the GameEnded event emits 0 as the prize amount instead of the actual pot value
function testVulnerability_GameEndedEventEmitsZeroPot() public {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
uint256 potBeforeDeclare = game.pot();
uint256 lastClaimTime = game.lastClaimTime();
uint256 gracePeriod = game.gracePeriod();
uint256 gameRound = game.gameRound();
address currentKing = game.currentKing();
uint256 expectedPot = (INITIAL_CLAIM_FEE * 95) / 100;
assertEq(potBeforeDeclare, expectedPot, "Pot should contain expected amount before declaring winner");
assertGt(potBeforeDeclare, 0, "Pot should be greater than 0 before declaring winner");
vm.warp(lastClaimTime + gracePeriod + 1 hours);
vm.expectEmit(true, false, false, false);
emit GameEnded(currentKing, 0, block.timestamp, gameRound);
game.declareWinner();
uint256 potAfterDeclare = game.pot();
assertEq(potAfterDeclare, 0, "Pot should be 0 after declaring winner");
uint256 winnerPendingWinnings = game.pendingWinnings(currentKing);
assertEq(winnerPendingWinnings, expectedPot, "Winner should have correct amount in pending winnings");
}
Recommended Mitigation
The code correctly captures prizeAmount before resetting pot. The event emits the correct prize amount.
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 prizeAmount = pot;
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
- emit GameEnded(currentKing, 0, block.timestamp, gameRound)
+ emit GameEnded(currentKing, prizeAmount, block.timestamp, gameRound)
}
