From 71d70dfb0efeadaa866db191385c40c169acd2d2 Mon Sep 17 00:00:00 2001 From: FloPinguin <25036848+FloPinguin@users.noreply.github.com> Date: Sat, 27 Jun 2026 20:10:24 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20prevent=20client=20from=20bypassing=20ra?= =?UTF-8?q?ndom=20spawn=20selection=20=F0=9F=9B=A1=EF=B8=8F=20(#4428)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description: When random spawn mode is active, players are supposed to receive randomly chosen spawns rather than choosing their own. However, `SpawnExecution.getSpawn()` checks `center !== undefined` first, which means if a player manually injects coordinates into the spawn intent (bypassing the client-side UI guard), the random selection logic is completely bypassed and the player gets their chosen coordinates. This was fully exploitable in singleplayer (where no pre-created `SpawnExecution` objects exist) and was a defense-in-depth gap in multiplayer (relying on execution order of pre-created spawns to block it via the `hasSpawned()` guard). The fix forces `center` to `undefined` in `getSpawn()` when random spawns are enabled, ensuring the random selection code path is always taken regardless of what the client sends. ## Changes: - `src/core/execution/SpawnExecution.ts`: Pass `undefined` to `getSpawn()` when `isRandomSpawn()` is true, ignoring any client-specified tile - `tests/core/execution/SpawnExecution.test.ts`: Added test verifying that a client-specified tile is ignored when random spawn is enabled ## Please complete the following: - [X] I have added screenshots for all UI updates - [X] I process any text displayed to the user through translateText() and I've added it to the en.json file - [X] I have added relevant tests to the test directory ## Please put your Discord username so you can be contacted if a bug or regression is found: FloPinguin --- src/core/execution/SpawnExecution.ts | 4 ++- tests/core/execution/SpawnExecution.test.ts | 27 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/core/execution/SpawnExecution.ts b/src/core/execution/SpawnExecution.ts index ec4d80974..10b7e963f 100644 --- a/src/core/execution/SpawnExecution.ts +++ b/src/core/execution/SpawnExecution.ts @@ -53,7 +53,9 @@ export class SpawnExecution implements Execution { } player.tiles().forEach((t) => player.relinquish(t)); - const spawn = this.getSpawn(this.tile); + const spawn = this.getSpawn( + this.mg.config().isRandomSpawn() ? undefined : this.tile, + ); if (!spawn) { console.warn(`SpawnExecution: cannot spawn ${this.playerInfo.name}`); diff --git a/tests/core/execution/SpawnExecution.test.ts b/tests/core/execution/SpawnExecution.test.ts index ea11e64e8..8f277ffcd 100644 --- a/tests/core/execution/SpawnExecution.test.ts +++ b/tests/core/execution/SpawnExecution.test.ts @@ -103,4 +103,31 @@ describe("Spawn execution", () => { // Previous territory from first spawn should be relinquished expect(game.owner(10).isPlayer()).toBe(false); }); + + test("Random spawn ignores client-specified tile", async () => { + const playerInfo = new PlayerInfo( + `player`, + PlayerType.Human, + `client_id`, + `player_id`, + ); + + const game = await setup("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.executeNextTick(); + game.executeNextTick(); + + const player = game.playerByClientID("client_id")!; + expect(player.hasSpawned()).toBe(true); + // The spawn tile should NOT be the client-specified tile — + // random spawn must bypass the client's choice. + expect(player.spawnTile()).not.toBe(maliciousTile); + expect(player.spawnTile()).toEqual(expect.any(Number)); + expect(game.isLand(player.spawnTile()!)).toBe(true); + }); });