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

'GameEnded' Event Emits incorrect amount Instead of Actual Winnings

nono

The GameEnded emits incorrect amount sent to Winner '0' instead of actual amount sent to the Winner.


Seveity : Low/Informational

Impact : Low, the GameEnded event emits the amount after it has been sent to '0'.

Description

  • The GameEnded Event Emits the amount of winnings sent to the Winner after it has been set to 0

  • While its not a threat to the Contract's Functionality but it can cause issues with Analytics cause its not emiting the exact amount sent to the Winner.

  • We want to emit the amount sent to Winner in the event to actually see how much he got, helps with Analytics instead of just 0.

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

  • Low/Informational as it can have reading issues when tryinng to see how much amount went to the Winner

Impact:

  • The GameEnded event emits the pot value after it has been reset to 0.
    As a result, the event always reports prizeAmount = 0, even though the actual prize was successfully transferred to the winner via pendingWinnings


Proof of Concept:

  • Deploy the contract and have a player claim the throne with 3 ETH.

  • Advance time beyond the grace period.

  • Call declareWinner().

  • Then the Contract will do:

pendingWinnings[currentKing] += pot;
pot = 0;
​
emit GameEnded(currentKing, pot, block.timestamp, gameRound); // <-- emits 0

Recommended Mitigation

What we are doing here in the code is saving the pot in prizeAmount Variable so that in the Event we emit it to see how much amount got sent to the Winner this helps in Analytics and to see how much money the Winner actually got.

- 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);}
​
+ 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, prizeAmount, block.timestamp, gameRound);
}
​
Updates

Appeal created

inallhonesty Lead Judge 15 days 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.