Root + Impact
Description
The GameEnded event is emitted after the pot is reset to zero, causing the event to report a prize amount of zero instead of the actual pot value.
@> pot = 0;
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
Risk
Likelihood:
Impact:
The emitted event incorrectly shows zero prize amount, breaking off-chain monitoring, analytics, or UI displays relying on event logs for prize info.
Proof of Concept
POc for Vuln. is not required as it is clearly visible that the pot is intialised to zero and event is emitted.
pragma solidity ^0.8.20;
import {Test, console2} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameTest is Test {
Game public game;
address public deployer;
address public player1;
uint256 public constant INITIAL_CLAIM_FEE = 0.1 ether;
uint256 public constant GRACE_PERIOD = 1 days;
uint256 public constant FEE_INCREASE_PERCENTAGE = 10;
uint256 public constant PLATFORM_FEE_PERCENTAGE = 5;
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.prank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
}
function test_claimmthrone() external{
vm.prank(player1);
game.claimThrone{value: 1 ether}();
vm.warp(80000000);
vm.prank(player1);
game.declareWinner();
assertEq(game.pot(),0);
}
}
Output: In the output we can see that the pot is emitted as zero.
[224004] GameTest::test_claimmthrone()
├─ [0] VM::prank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [152621] Game::claimThrone{value: 1000000000000000000}()
│ ├─ emit ThroneClaimed(newKing: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], claimAmount: 1000000000000000000 [1e18], newClaimFee: 110000000000000000 [1.1e17], newPot: 950000000000000000 [9.5e17], timestamp: 1)
│ └─ ← [Stop]
├─ [0] VM::warp(80000000 [8e7])
│ └─ ← [Return]
├─ [0] VM::prank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [50689] Game::declareWinner()
│ ├─ emit GameEnded(winner: player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84], prizeAmount: 0, timestamp: 80000000 [8e7], round: 1)
│ └─ ← [Stop]
├─ [515] Game::pot() [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::assertEq(0, 0) [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Recommended Mitigation
By intialsing the pot to another variable and using prizeamount variable in the event emission will mitigate the issue.
+ uint256 prizeamount = pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, prizeamount , block.timestamp, gameRound);
