Root + Impact
Description
In the declareWinner() function, the GameEnded event emits the pot value after it has already been reset to 0. This leads to incorrect and misleading event logs, where the emitted pot amount appears as zero — despite the winner actually receiving a non-zero reward.
Risk
Likelihood:
Impact:
-
Potential disputes: If players are tracking their rewards based on event data (e.g., UIs, bots, or analytics dashboards), they could wrongly assume rewards weren't issued
-
Incorrect event logs: Off-chain services, indexers, and users relying on event data will believe that the winner received 0 ETH.
Proof of Concept
pragma solidity ^0.8.20;
import {Test, console} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameTest is Test {
Game public game;
address public deployer;
address public player1;
address public player2;
address public player3;
address public maliciousActor;
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");
player2 = makeAddr("player2");
player3 = makeAddr("player3");
maliciousActor = makeAddr("maliciousActor");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.deal(player2, 10 ether);
vm.deal(player3, 10 ether);
vm.deal(maliciousActor, 10 ether);
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
modifier loadGame() {
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
console.log(game.currentKing());
vm.startPrank(player2);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
_;
}
function testDeclareWinner() public loadGame {
vm.warp(GRACE_PERIOD + 1 hours);
vm.roll(block.number + 3);
vm.prank(player3);
game.declareWinner();
}
}
Result trace
prizeAmount is emitted as 0.
├─ [0] VM::prank(player3: [0xcC37919fDb8E2949328cDB49E8bAcCb870d0c9f3])
│ └─ ← [Return]
├─ [48689] Game::declareWinner()
│ ├─ emit GameEnded(winner: player2: [0xEb0A3b7B96C1883858292F0039161abD287E3324], prizeAmount: 0, timestamp: 90000 [9e4], round: 1)
│ └─ ← [Stop]
└─ ← [Stop]
Recommended Mitigation
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
// @audit:: emmitted event is wrong
// @audit::cache uint256 prizeAmount = pot;
- emit GameEnded(currentKing, pot, block.timestamp, gameRound);
+ emit GameEnded(currentKing, prizeAmount, block.timestamp, gameRound);
}
