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

Event Game::GameEnded emits total prize amount in Game::declareWinner as Zero which creates a false assumption that the total prize is zero

0xtory

Root + Impact

Description

  • After a game round has finished , and someone tries to call Game::declareWinner ,an event Game::GameEnded is emitted which includes the currentKing, prizeAmount , timestamp, and currentRound, but the position for prizeAmount would always emit as zero because the protocol is resetting total amount in pot to zero and then emit the value, this will lead to a misunderstanding for frontend applications listening for these events

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 would occur everytime anyone calls the Game::declareWinner function

Impact:

  • Frontend Applications listening for these events would recieve a log with a prizeAmount of zero everytime a winner is declared

Proof of Concept

When the game is being concluded and someone calls Game::declareWInner prize amount is emitted as zero

function testEventEmitsPrizeAmountAsZero() public {
vm.startPrank(player1);
// Someone claims throne,waits for time to elapse then declare winner but events emits prizeAmount as zero
game.claimThrone{value: 3 ether}();
​
vm.warp(block.timestamp + 1 days + 3);
​
vm.expectEmit(true, true, true, true);
emit GameEnded(player1, 0, block.timestamp, 1);
​
game.declareWinner();
​
assertEq(game.pot(), 0);
}
​
Results:
​
├─ [0] VM::warp(86404 [8.64e4])
│ └─ ← [Return]
├─ [0] VM::expectEmit(true, true, true, true)
│ └─ ← [Return]
├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 0, timestamp: 86404 [8.64e4], round: 1)
├─ [50689] Game::declareWinner()
│ ├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 0, timestamp: 86404 [8.64e4], round: 1)
│ └─ ← [Stop]
├─ [515] Game::pot() [staticcall]
│ └─ ← [Return] 0
└─ ← [Stop]

Recommended Mitigation

The function Game::declareWinner should store total pot amount in a local variable before resetting pot amount , the variable would be emitted instead

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 currentPot = pot;
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
​
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, currentPot, block.timestamp, gameRound)
}
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.