From 2bdf876b47c5e3134cc26a9d43a4b3d61c6c924d Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:40:43 +0100 Subject: [PATCH] fix(multiplayer): enforce assignment opaque ids --- Game/scripts/assignment_state.gd | 9 ++++++++- Game/scripts/control_plane_client.gd | 2 +- Game/tests/cases/test_assignment_state.gd | 10 ++++++++-- Game/tests/cases/test_control_plane_client.gd | 5 ++++- multiplayer-next.md | 2 ++ 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Game/scripts/assignment_state.gd b/Game/scripts/assignment_state.gd index 20a9dea4..d61a9402 100644 --- a/Game/scripts/assignment_state.gd +++ b/Game/scripts/assignment_state.gd @@ -32,7 +32,7 @@ func apply(payload: Dictionary, expected_player_id: String = "") -> bool: if not is_valid_expiry_timestamp(next_expires_at): return _reject("Assignment response contains invalid expiry") var expiry_unix := Time.get_unix_time_from_datetime_string(next_expires_at) - if next_match_id.is_empty() or next_server_id.is_empty() or next_player_id.is_empty() or (not expected_player_id.is_empty() and next_player_id != expected_player_id) or int(payload["slot"]) < 0 or int(payload["slot"]) > 5 or int(payload["protocol_version"]) < 1 or (next_transport != "enet" and next_transport != "steam_sdr") or not _valid_endpoint(next_endpoint) or expiry_unix <= Time.get_unix_time_from_system() or String(payload["join_authorisation"]).is_empty(): + if not is_valid_opaque_id(next_match_id) or not is_valid_opaque_id(next_server_id) or not is_valid_opaque_id(next_player_id) or (not expected_player_id.is_empty() and next_player_id != expected_player_id) or int(payload["slot"]) < 0 or int(payload["slot"]) > 5 or int(payload["protocol_version"]) < 1 or (next_transport != "enet" and next_transport != "steam_sdr") or not _valid_endpoint(next_endpoint) or expiry_unix <= Time.get_unix_time_from_system() or String(payload["join_authorisation"]).is_empty(): return _reject("Assignment response contains invalid values") match_id = next_match_id server_id = next_server_id @@ -54,6 +54,13 @@ static func is_valid_expiry_timestamp(value: String) -> bool: return timestamp_pattern.search(value) != null +static func is_valid_opaque_id(value: String) -> bool: + if value.length() < 16 or value.length() > 128: + return false + var resource_pattern := RegEx.create_from_string("^[A-Za-z0-9_-]+$") + return resource_pattern.search(value) != null + + static func _valid_endpoint(value: String) -> bool: if value.is_empty() or value.contains("/") or value.contains("?") or value.contains("#"): return false diff --git a/Game/scripts/control_plane_client.gd b/Game/scripts/control_plane_client.gd index 917624f3..c4ba8cc2 100644 --- a/Game/scripts/control_plane_client.gd +++ b/Game/scripts/control_plane_client.gd @@ -493,7 +493,7 @@ static func _valid_websocket_event(event: Dictionary) -> bool: return false var event_name := String(event["event"]) if event_name == "assignment_changed": - return event.has("match_id") and event["match_id"] is String and not String(event["match_id"]).is_empty() and event.has("server_id") and event["server_id"] is String and not String(event["server_id"]).is_empty() + return event.has("match_id") and event["match_id"] is String and is_valid_resource_id(String(event["match_id"])) and event.has("server_id") and event["server_id"] is String and is_valid_resource_id(String(event["server_id"])) if event_name == "error": return event.has("code") and String(event["code"]) in ["REVISION_GAP", "NOT_AUTHORISED", "INVALID_STATE", "RATE_LIMITED"] if event_name == "state_changed": diff --git a/Game/tests/cases/test_assignment_state.gd b/Game/tests/cases/test_assignment_state.gd index 2aa0c4d4..7d002080 100644 --- a/Game/tests/cases/test_assignment_state.gd +++ b/Game/tests/cases/test_assignment_state.gd @@ -5,7 +5,7 @@ const AssignmentState = preload("res://scripts/assignment_state.gd") func test_assignment_projection_accepts_verified_enet_manifest() -> void: var assignment := AssignmentState.new() - assert_true(assignment.apply({"match_id": "match-1", "server_id": "server-1", "player_id": "player-1", "slot": 2, "expires_at": "2099-08-31T12:00:00Z", "protocol_version": 1, "transport": "enet", "endpoint": "127.0.0.1:30001", "join_authorisation": "signed"}, "player-1"), "valid assignment applies") + assert_true(assignment.apply({"match_id": "match_1234567890", "server_id": "server_123456789", "player_id": "player_123456789", "slot": 2, "expires_at": "2099-08-31T12:00:00Z", "protocol_version": 1, "transport": "enet", "endpoint": "127.0.0.1:30001", "join_authorisation": "signed"}, "player_123456789"), "valid assignment applies") assert_true(assignment.available, "assignment becomes available only after validation") assert_eq(assignment.transport, "enet", "transport is explicit") assert_eq(assignment.slot, 2, "slot is preserved") @@ -14,7 +14,13 @@ func test_assignment_projection_accepts_verified_enet_manifest() -> void: func test_assignment_projection_rejects_wrong_shape_or_unsafe_transport() -> void: var assignment := AssignmentState.new() - assert_true(not assignment.apply({"match_id": "match-1", "server_id": "server-1", "player_id": "player-1", "slot": 6, "expires_at": "future", "protocol_version": 1, "transport": "enet", "join_authorisation": "signed"}), "out-of-range slot is rejected") + var valid := {"match_id": "match_1234567890", "server_id": "server_123456789", "player_id": "player_123456789", "slot": 0, "expires_at": "2099-08-31T12:00:00Z", "protocol_version": 1, "transport": "enet", "endpoint": "127.0.0.1:30001", "join_authorisation": "signed"} + var out_of_range := valid.duplicate() + out_of_range["slot"] = 6 + assert_true(not assignment.apply(out_of_range), "out-of-range slot is rejected") + var short_id := valid.duplicate() + short_id["match_id"] = "match-1" + assert_true(not assignment.apply(short_id), "short opaque assignment id is rejected") assert_true(not assignment.available, "invalid assignment is not exposed") assert_true(not assignment.apply({"match_id": "match-1", "server_id": "server-1", "player_id": "player-1", "slot": 0, "expires_at": "future", "protocol_version": 1, "transport": "udp", "join_authorisation": "signed"}), "unknown transport is rejected") assert_true(not assignment.apply({"match_id": "match-1", "server_id": "server-1", "player_id": "player-1", "slot": 0, "expires_at": "future", "protocol_version": 1, "transport": "steam_sdr", "join_authorisation": ""}), "empty authorisation is rejected") diff --git a/Game/tests/cases/test_control_plane_client.gd b/Game/tests/cases/test_control_plane_client.gd index 4f594f08..0fb167ed 100644 --- a/Game/tests/cases/test_control_plane_client.gd +++ b/Game/tests/cases/test_control_plane_client.gd @@ -75,8 +75,11 @@ func test_websocket_event_validation_requires_contract_specific_fields() -> void var bad_state := envelope.duplicate() bad_state["state"] = "SECRET" assert_true(not ControlPlaneClient._valid_websocket_event(bad_state), "unknown state event is rejected") - var assignment := {"event": "assignment_changed", "revision": 0, "resource_id": "match_1234567890", "occurred_at": "2026-08-31T12:00:00Z", "match_id": "match-1", "server_id": "server-1"} + var assignment := {"event": "assignment_changed", "revision": 0, "resource_id": "match_1234567890", "occurred_at": "2026-08-31T12:00:00Z", "match_id": "match_1234567890", "server_id": "server_123456789"} assert_true(ControlPlaneClient._valid_websocket_event(assignment), "complete assignment event is accepted") + var short_assignment_id := assignment.duplicate() + short_assignment_id["server_id"] = "server-1" + assert_true(not ControlPlaneClient._valid_websocket_event(short_assignment_id), "short assignment server id is rejected") assignment.erase("server_id") assert_true(not ControlPlaneClient._valid_websocket_event(assignment), "incomplete assignment event is rejected") var fractional := envelope.duplicate() diff --git a/multiplayer-next.md b/multiplayer-next.md index 1fdf3fa8..3e8a5e86 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1589,6 +1589,8 @@ Ranked profile projection now rejects fractional `ranked_games` values instead o Ranked profile projection now enforces the OpenAPI tier enum, rejecting unknown tier labels before they reach the HUD. +Assignment projections and assignment-changed events now enforce the published opaque-ID shape for match, server, and player identifiers; short or unsafe IDs fail closed. + Signed MatchNet claims now also require exact JSON string/integer types for every identity, protocol, expiry, slot, team, and generation field; string-number coercion is rejected before canonical signature verification. Presentation progress: a shared `Game/themes/cosmic_clash_theme.tres` now gives the menu, lobby, matchmaking, and settings surfaces consistent button, input, option, and label styling. The custom-font portion of `TODO.md` remains open until a distributable font asset is selected.