mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-30 14:02:12 +00:00
fix: prevent client from bypassing random spawn selection 🛡️ (#4428)
## 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
This commit is contained in:
@@ -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}`);
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user