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

Reset Of `pot` To Zero In `declareWinner()` Results In Misleading Event Log

sneekystryder

Reset Of pot To Zero In declareWinner() Results In Misleading Event Log

Description

  • When the game ends, the contract is expected to emit a GameEnded event containing the address of the winner and the prize amount they won from the pot. This event helps off-chain systems and users track who won and how much.

  • However, the pot is reset to zero before emitting the GameEnded event, which results in a prize amount of 0 being logged. This creates misleading historical records and makes it appear as though the winner received no reward, even though the pot may have been correctly transferred to pendingWinnings.

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);
}

Risk

Likelihood:

  • This will occur every time a winner is declared through declareWinner(), because the pot is reset to 0 before the GameEnded event is emitted.

  • It will affect all rounds across the entire lifecycle of the contract.

Impact:

  • Off-chain systems and user interfaces relying on event logs will display inaccurate or misleading prize data.

  • Winners may appear to have won nothing, which damages the credibility of the contract and reduces confidence in its transparency.

Proof of Concept

Add this test to Game.t.sol and run with forge test —mt testGameEndedEmitsZeroPot

function testGameEndedEmitsZeroPot() public {
// Player 1 claims throne and becomes King
vm.startPrank(player1);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
​
// 1 day passes
vm.warp(block.timestamp + 1 days + 1);
​
// Winner is declared and GameEnded event is emitted
vm.recordLogs();
game.declareWinner();
​
Vm.Log[] memory entries = vm.getRecordedLogs();
​
// Logs decoded
bytes32 topic0 = entries[0].topics[0];
assertEq(topic0, keccak256("GameEnded(address,uint256,uint256,uint256)"));
(bytes memory data) = entries[0].data;
(uint256 prize, , ) = abi.decode(data, (uint256, uint256, uint256));
​
// Prize in log is shown as 0
assertEq(prize, 0);
}
​

Recommended Mitigation

Emit the value of pot by creating a prize variable, setting it to the value of pot, and emitting it in the GameEnded event

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;
+ pendingWinnings[currentKing] += prize;
+ pot = 0;
+
+ emit GameEnded(currentKing, prize, 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!