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

Wrong value for prizeAmount in GameEnded event

neomartis

Wrong value for prizeAmount in GameEnded event

Description

  • The GameEnded event is emitted with the values of the currentKing, pot/prize, timestamp and gameRound

  • However, the value of pot/prize will always be 0 as the value is reset before the event is emitted.

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: High

  • This will always happen with the GameEnded event as the pot is set to 0 on the line before the event is emitted

Impact: Low

  • The game is still valid, and the winner actually does receive the correct amount as price

  • Any scanners, apps/front-end or indexing based on the events will not work as expected as the prize amount will always be 0

Proof of Concept

To validate the issue, we can run the following test (with the default setup as per Game.t.sol)

function testIssueShouldEventGameEndedEvent() public {
// Claim for the first time
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
​
vm.warp(block.timestamp + GRACE_PERIOD + 1);
vm.prank(deployer);
​
console2.log("game.pot()", game.pot());
​
vm.expectEmit(true, true, true, true);
emit GameEnded(player1, game.pot(), block.timestamp, 1);
game.declareWinner();
assertEq(game.gameEnded(), true);
​
}

logs

├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 95000000000000000 [9.5e16], timestamp: 86402 [8.64e4], round: 1)
├─ [50689] Game::declareWinner()
│ ├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 0, timestamp: 86402 [8.64e4], round: 1)
│ └─ ← [Stop]
└─ ← [Revert] log != expected log

As can be seen in the last lines, we emit the wrong value, where we expect 9.5e16 but we emit 0:

Recommended Mitigation

This issue can be fixed by removing the pot = 0. Note that this will work because:

  • pot will be set to 0 when resetGame is called

  • Any functionality to the pot (claimThrone and declareWinner) cannot be called when gameEnded is true

  • gameEnded is set to true only in resetGame
    Meaning that we can safely remove the pot = 0 line in declareWinner()

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