diff --git a/src/core/execution/ExecutionManager.ts b/src/core/execution/ExecutionManager.ts index ccdb792d69..a2a9cbac08 100644 --- a/src/core/execution/ExecutionManager.ts +++ b/src/core/execution/ExecutionManager.ts @@ -70,7 +70,12 @@ export class Executor { case "move_warship": return new MoveWarshipExecution(player, intent.unitIds, intent.tile); case "spawn": - return new SpawnExecution(this.gameID, player.info(), intent.tile); + return new SpawnExecution( + this.gameID, + player.info(), + intent.tile, + true, + ); case "boat": return new TransportShipExecution(player, intent.dst, intent.troops); case "allianceRequest": diff --git a/src/core/execution/SpawnExecution.ts b/src/core/execution/SpawnExecution.ts index c554f873c7..6e4b7fbe88 100644 --- a/src/core/execution/SpawnExecution.ts +++ b/src/core/execution/SpawnExecution.ts @@ -27,6 +27,10 @@ export class SpawnExecution implements Execution { gameID: GameID, private playerInfo: PlayerInfo, public tile?: TileRef, + // True when this spawn came from a client "spawn" intent (a player choosing + // where to spawn). Internal spawns — nations, bots, random-spawn placement — + // leave this false and are never gated by the spawn phase. + private fromIntent: boolean = false, ) { this.random = new PseudoRandom( simpleHash(playerInfo.id) + simpleHash(gameID), @@ -40,6 +44,19 @@ export class SpawnExecution implements Execution { tick(ticks: number) { this.active = false; + // Security: a client-requested spawn is only valid during the spawn phase. + // Once the phase has ended, ignore the intent so a player can neither spawn + // for the first time nor re-spawn mid-game. This closes the "teleport" + // exploit, where a crafted spawn intent relinquished a player's territory + // and re-conquered it elsewhere. Internal spawns (nations, bots, random + // spawn placement) are queued during the spawn phase by trusted code and + // may land a tick later — they set fromIntent=false and are not gated here. + // inSpawnPhase() is deterministic game state, so the intent is an identical + // no-op on every client. + if (this.fromIntent && !this.mg.inSpawnPhase()) { + return; + } + let player: Player | null = null; if (this.mg.hasPlayer(this.playerInfo.id)) { player = this.mg.player(this.playerInfo.id); @@ -47,15 +64,6 @@ export class SpawnExecution implements Execution { player = this.mg.addPlayer(this.playerInfo); } - // Security: a spawn intent may only place or relocate a player's starting - // territory during the spawn phase. Once the game is underway, an - // already-spawned player who sends a spawn intent is attempting to - // teleport — relinquishing their territory and re-conquering it elsewhere. - // Ignore it so the intent is a deterministic no-op on every client. - if (!this.mg.inSpawnPhase() && player.hasSpawned()) { - return; - } - // Security: If random spawn is enabled, prevent players from re-rolling their spawn location if (this.mg.config().isRandomSpawn() && player.hasSpawned()) { return; diff --git a/tests/core/execution/ExecutionManager.test.ts b/tests/core/execution/ExecutionManager.test.ts new file mode 100644 index 0000000000..75d3dcb6c3 --- /dev/null +++ b/tests/core/execution/ExecutionManager.test.ts @@ -0,0 +1,37 @@ +import { Executor } from "../../../src/core/execution/ExecutionManager"; +import { SpawnExecution } from "../../../src/core/execution/SpawnExecution"; +import { PlayerInfo, PlayerType } from "../../../src/core/game/Game"; +import { StampedIntent } from "../../../src/core/Schemas"; +import { setup } from "../../util/Setup"; + +describe("Executor.createExec", () => { + test("marks a client spawn intent with fromIntent so it is gated by the spawn phase", async () => { + const playerInfo = new PlayerInfo( + "player", + PlayerType.Human, + "client_id", + "player_id", + ); + // setup() ends the spawn phase by default, so the game is already underway. + const game = await setup("half_land_half_ocean", {}, [playerInfo]); + + const executor = new Executor(game, "game_id", "client_id"); + const exec = executor.createExec({ + type: "spawn", + tile: 20, + clientID: "client_id", + } as StampedIntent); + + expect(exec).toBeInstanceOf(SpawnExecution); + expect((exec as SpawnExecution).tile).toBe(20); + + // Because it originated from a client intent (fromIntent = true), executing + // it after the spawn phase must be a no-op — the player never spawns. + game.addExecution(exec); + game.executeNextTick(); + game.executeNextTick(); + expect(game.playerByClientID("client_id")?.hasSpawned() ?? false).toBe( + false, + ); + }); +}); diff --git a/tests/core/execution/SpawnExecution.test.ts b/tests/core/execution/SpawnExecution.test.ts index 4671c7c194..a91b030426 100644 --- a/tests/core/execution/SpawnExecution.test.ts +++ b/tests/core/execution/SpawnExecution.test.ts @@ -1,6 +1,22 @@ +import path from "path"; import { SpawnExecution } from "../../../src/core/execution/SpawnExecution"; import { PlayerInfo, PlayerType } from "../../../src/core/game/Game"; +import { GameConfig } from "../../../src/core/Schemas"; import { setup } from "../../util/Setup"; +import { TestConfig } from "../../util/TestConfig"; + +// tests/util, so Setup resolves its map paths relative to it (../testdata/maps). +const UTIL_DIR = path.join(__dirname, "..", "..", "util"); + +// Keep the game in the spawn phase (setup() ends it by default), for asserting +// that a client-requested spawn is accepted while the phase is open. +function setupSpawnPhase( + mapName: string, + gameConfig: Partial, + players: PlayerInfo[], +) { + return setup(mapName, gameConfig, players, UTIL_DIR, TestConfig, false); +} describe("Spawn execution", () => { // Manually calculated based on number of tiles in manifest of each map @@ -94,16 +110,39 @@ describe("Spawn execution", () => { const game = await setup("half_land_half_ocean", {}, [playerInfo]); + game.addExecution(new SpawnExecution("game_id", playerInfo, 10)); game.addExecution(new SpawnExecution("game_id", playerInfo, 20)); game.executeNextTick(); game.executeNextTick(); + expect(game.playerByClientID("client_id")?.spawnTile()).toBe(20); + // Previous territory from first spawn should be relinquished + expect(game.owner(10).isPlayer()).toBe(false); + }); + + test("Client spawn intent is accepted during the spawn phase", async () => { + const playerInfo = new PlayerInfo( + `player`, + PlayerType.Human, + `client_id`, + `player_id`, + ); + + const game = await setupSpawnPhase("half_land_half_ocean", {}, [ + playerInfo, + ]); + + // fromIntent = true simulates a client "spawn" intent. + game.addExecution(new SpawnExecution("game_id", playerInfo, 20, true)); + game.executeNextTick(); + game.executeNextTick(); + const player = game.playerByClientID("client_id")!; expect(player.spawnTile()).toBe(20); expect(player.numTilesOwned()).toBeGreaterThan(0); }); - test("Spawn intent after the spawn phase cannot relocate territory (anti-teleport)", async () => { + test("Client spawn intent after the spawn phase is ignored (anti-teleport)", async () => { const playerInfo = new PlayerInfo( `player`, PlayerType.Human, @@ -114,7 +153,7 @@ describe("Spawn execution", () => { // setup() ends the spawn phase by default, so the game is already underway. const game = await setup("half_land_half_ocean", {}, [playerInfo]); - // Establish the player's territory with a legitimate first spawn. + // Establish the player's territory (an internal/trusted spawn). game.addExecution(new SpawnExecution("game_id", playerInfo, 20)); game.executeNextTick(); game.executeNextTick(); @@ -124,10 +163,10 @@ describe("Spawn execution", () => { const tilesBefore = player.numTilesOwned(); expect(tilesBefore).toBeGreaterThan(0); - // Malicious "teleport": a spawn intent to a new tile after the game has - // started must be a deterministic no-op — the player keeps their original + // Malicious "teleport": a client spawn intent to a new tile after the phase + // ended must be a deterministic no-op — the player keeps their original // spawn location and territory rather than relinquishing and re-conquering. - game.addExecution(new SpawnExecution("game_id", playerInfo, 10)); + game.addExecution(new SpawnExecution("game_id", playerInfo, 10, true)); game.executeNextTick(); game.executeNextTick(); @@ -135,6 +174,49 @@ describe("Spawn execution", () => { expect(player.numTilesOwned()).toBe(tilesBefore); }); + test("Client spawn intent after the spawn phase cannot spawn a never-spawned player", async () => { + const playerInfo = new PlayerInfo( + `player`, + PlayerType.Human, + `client_id`, + `player_id`, + ); + + const game = await setup("half_land_half_ocean", {}, [playerInfo]); + + // A client "spawn" intent once the phase has ended is ignored entirely. + game.addExecution(new SpawnExecution("game_id", playerInfo, 20, true)); + game.executeNextTick(); + game.executeNextTick(); + + const player = game.playerByClientID("client_id"); + expect(player?.hasSpawned() ?? false).toBe(false); + expect(game.owner(20).isPlayer()).toBe(false); + }); + + test("Internal spawns are not gated by the spawn phase", async () => { + // Nations, bots and random-spawn placement queue a SpawnExecution during + // the spawn phase, but it may land a tick after the phase ends (e.g. a + // singleplayer human's spawn ends the phase early). These trusted spawns + // (fromIntent = false) must still succeed. + const playerInfo = new PlayerInfo( + `player`, + PlayerType.Human, + `client_id`, + `player_id`, + ); + + const game = await setup("half_land_half_ocean", {}, [playerInfo]); + + game.addExecution(new SpawnExecution("game_id", playerInfo, 20)); + game.executeNextTick(); + game.executeNextTick(); + + const player = game.playerByClientID("client_id")!; + expect(player.spawnTile()).toBe(20); + expect(player.numTilesOwned()).toBeGreaterThan(0); + }); + test("Random spawn ignores client-specified tile", async () => { const playerInfo = new PlayerInfo( `player`, @@ -143,13 +225,17 @@ describe("Spawn execution", () => { `player_id`, ); - const game = await setup("half_land_half_ocean", { randomSpawn: true }, [ - playerInfo, - ]); + const game = await setupSpawnPhase( + "half_land_half_ocean", + { randomSpawn: true }, + [playerInfo], + ); // Simulate a malicious client sending a spawn intent with a specific tile const maliciousTile = 10; - game.addExecution(new SpawnExecution("game_id", playerInfo, maliciousTile)); + game.addExecution( + new SpawnExecution("game_id", playerInfo, maliciousTile, true), + ); game.executeNextTick(); game.executeNextTick();