Root + Impact
Description
The Game.sol contract has a constructor with 4 parameters, none of which is for setting the value for the currentKing state variable. This means, when deployed, the currentKing remains address(0), which is the default value for the address data type.
address public currentKing;
Moreover, if the attacker is the owner, there is no way he can set the currentKing to another value with the defined functions in the contract. This issue makes the game unplayable because nobody can invoke claimThrone without triggering the second require condition:
require(
msg.sender == currentKing,
"Game: You are already the king. No need to re-claim."
);
As far as I know, if the attacker is not the owner of the game, there is no way to use address(0) as msg.sender, even when invoked in a constructor of an attack contract like this:
pragma solidity ^0.8.0;
import {console} from "forge-std/console.sol";
import {Game} from "./Game.sol";
contract Attack {
constructor(address _game) payable {
console.log("attack's address: ", address(this));
Game(payable(_game)).claimThrone{value: 1 ether}();
}
}
pragma solidity ^0.8.0;
import {Script} from "forge-std/Script.sol";
import {GameScript} from "./Game.s.sol";
import {Game} from "../src/Game.sol";
import {Attack} from "../src/Attack.sol";
import {Vm} from "forge-std/Vm.sol";
contract AttackScript is Script {
GameScript gameScript;
Game game;
Attack attack;
function run() public {
gameScript = new GameScript();
game = gameScript.run();
vm.startBroadcast(0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266);
attack = new Attack{value: 1 ether}(address(game));
vm.stopBroadcast();
}
}
PoC
Scenario 1
Claim the throne as the owner
❯ forge script .\script\Game.s.sol --rpc-url http://localhost:8545 --private-key 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 --broadcast
[⠊] Compiling...
No files changed, compilation skipped
Script ran successfully.
== Return ==
0: contract Game 0x5FbDB2315678afecb367f032d93F642f64180aa3
== Logs ==
Game contract deployed to: 0x5FbDB2315678afecb367f032d93F642f64180aa3
Initial Claim Fee: 10000000000000000
Grace Period (seconds): 259200
Fee Increase Percentage: 15
Platform Fee Percentage: 3
#
==========================
Chain 31337
Estimated gas price: 2.000000001 gwei
Estimated total gas used for script: 2901762
Estimated amount required: 0.005803524002901762 ETH
==========================
#
✅ [Success] Hash: 0x9b2bd8af93fdb5f0d903acb977dce6948a9083885248b703eebe0277534bdfed
Contract Address: 0x5FbDB2315678afecb367f032d93F642f64180aa3
Block: 1
Paid: 0.002232125002232125 ETH (2232125 gas * 1.000000001 gwei)
✅ Sequence #1 on anvil-hardhat | Total Paid: 0.002232125002232125 ETH (2232125 gas * avg 1.000000001 gwei)
==========================
ONCHAIN EXECUTION COMPLETE & SUCCESSFUL.
Transactions saved to: D:/repos/last-man-standing/broadcast\Game.s.sol\31337\run-latest.json
Sensitive values saved to: D:/repos/last-man-standing/cache\Game.s.sol\31337\run-latest.json
❯ cast send 0x5FbDB2315678afecb367f032d93F642f64180aa3 "claimThrone()()" --value "1 ether" --rpc-url http://localhost:8545 --private-key 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
Error: Failed to estimate gas: server returned an error response: error code 3: execution reverted: Game: You are already the king. No need to re-claim., data: "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003447616d653a20596f752061726520616c726561647920746865206b696e672e204e6f206e65656420746f2072652d636c61696d2e000000000000000000000000": Error("Game: You are already the king. No need to re-claim.")
Scenario 2
Claim the throne while constructing a contract (the Attack contract above).
❯ forge create .\src\Attack.sol:Attack --rpc-url http://localhost:8545 --private-key 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80 --broadcast --value "1 ether" --constructor-args 0x5FbDB2315678afecb367f032d93F642f64180aa3 -vvvvv
[⠊] Compiling...
No files changed, compilation skipped
Error: server returned an error response: error code 3: execution reverted: Game: You are already the king. No need to re-claim., data: "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003447616d653a20596f752061726520616c726561647920746865206b696e672e204e6f206e65656420746f2072652d636c61696d2e000000000000000000000000"
Recommended Mitigation
Consider initialising the currentKing variable with a controlled address (can be EOA or smart contract address).
