Root + Impact
Description
-
Describe the normal behavior in one or more sentences:
After initializing the contract, currentKing is set to a null address by default. Each new player who raises the bet must become the king instead of the previous one.
-
Explain the specific issue or problem in one or more sentences:
When the claimThrone() function is called, the "You are already the king" check occurs, in which a logical error is made. In the current implementation, no one will be able to call this function.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.")
@> require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
Risk
Likelihood:
No one will be able to play the game.
Impact:
The basic functionality of the contract is broken.
Proof of Concept
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;
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");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
function testClaimThrone_invalidRequire() public {
address currentAddressKing = game.currentKing();
emit log_named_address("Current address king:", currentAddressKing);
vm.startPrank(player1);
game.claimThrone{value: 1 ether}();
vm.stopPrank();
}
}
Return test case:
[49896] GameTest::testClaimThrone_invalidRequire()
├─ [2618] Game::currentKing() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ emit log_named_address(key: "Current address king:", val: 0x0000000000000000000000000000000000000000)
├─ [0] VM::startPrank(player1: [0x7026B763CBE7d4E72049EA67E89326432a50ef84])
│ └─ ← [Return]
├─ [27259] Game::claimThrone{value: 1000000000000000000}()
│ └─ ← [Revert] Game: You are already the king. No need to re-claim.
└─ ← [Revert] Game: You are already the king. No need to re-claim.
Recommended Mitigation
An error in address comparison needs to be fixed.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.")
- require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
+ require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
