From c6d159d5f406404b635f109265c7cb7063aae906 Mon Sep 17 00:00:00 2001
From: ddidderr <ddidderr@paul.network>
Date: Wed, 2 Sep 2026 22:27:39 +0200
Subject: [PATCH] fix(proto): reject separators and control chars in wire game
 IDs

Security audit finding EXP2-SEC-04. `validate_game_id` in the wire
protocol only enforced non-blank and a 255-byte maximum, so a request
such as `StreamInstall { game_id: "../../x" }` decoded successfully and
was passed on to the transfer layer. Every consumer resolves the ID
against the local catalog before touching the filesystem, so this was
not exploitable, but the protocol boundary is the right place to state
what a game ID is: the name of one catalog directory.

Requests and library snapshots now fail validation when the game ID
contains `/`, `\`, any Unicode control character (including NUL), or is
exactly `.` or `..`. A new `ControlValidationError::InvalidPathComponent`
variant reports the rejection. Embedded dots such as "game..v1" remain
valid because the catalog validators accept them.

Test plan: `just test` exercises the rejected forms plus an accepted ID
with embedded dots.

Claude-Session: https://claude.ai/code/session_017C3Nbgwpdm3YNwZhhFLHwg
---
 crates/lanspread-proto/src/lib.rs | 47 +++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/crates/lanspread-proto/src/lib.rs b/crates/lanspread-proto/src/lib.rs
index bb28c03..d709d61 100644
--- a/crates/lanspread-proto/src/lib.rs
+++ b/crates/lanspread-proto/src/lib.rs
@@ -635,6 +635,9 @@ pub enum ControlValidationError {
         field: &'static str,
         maximum: usize,
     },
+    InvalidPathComponent {
+        field: &'static str,
+    },
     DuplicateGameId,
     UnsortedGameIds,
 }
@@ -657,6 +660,9 @@ impl fmt::Display for ControlValidationError {
             Self::TooManyItems { field, maximum } => {
                 write!(formatter, "{field} exceeds its {maximum}-item limit")
             }
+            Self::InvalidPathComponent { field } => {
+                write!(formatter, "{field} is not a single plain path component")
+            }
             Self::DuplicateGameId => formatter.write_str("library contains a duplicate game ID"),
             Self::UnsortedGameIds => {
                 formatter.write_str("library game IDs are not strictly sorted")
@@ -909,6 +915,10 @@ impl CallToPlayAuthorEvent {
     }
 }
 
+/// Game IDs name one catalog directory. The wire boundary therefore admits
+/// only a single plain path component: no separators, no control characters,
+/// and neither of the `.`/`..` pseudo-components. Every consumer still resolves
+/// the ID against the local catalog before touching the filesystem.
 fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
     if game_id.trim().is_empty() {
         return Err(ControlValidationError::EmptyField { field: "game ID" });
@@ -919,6 +929,13 @@ fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
             maximum: MAX_GAME_ID_BYTES,
         });
     }
+    if matches!(game_id, "." | "..")
+        || game_id
+            .chars()
+            .any(|character| character.is_control() || matches!(character, '/' | '\\'))
+    {
+        return Err(ControlValidationError::InvalidPathComponent { field: "game ID" });
+    }
     Ok(())
 }
 
@@ -1530,6 +1547,36 @@ mod tests {
             .encode()
             .is_err()
         );
+        for game_id in [
+            ".",
+            "..",
+            "../game",
+            "game/../other",
+            "dir/game",
+            "dir\\game",
+            "game\0",
+            "game\n",
+        ] {
+            assert!(
+                matches!(
+                    Request::StreamInstall {
+                        game_id: game_id.to_owned(),
+                        content_id: content(1),
+                    }
+                    .encode(),
+                    Err(ControlCodecError::Invalid(
+                        ControlValidationError::InvalidPathComponent { field: "game ID" }
+                    ))
+                ),
+                "accepted game ID {game_id:?}"
+            );
+        }
+        Request::StreamInstall {
+            game_id: "game..v1 (final)".to_owned(),
+            content_id: content(1),
+        }
+        .encode()
+        .expect("plain component with embedded dots should encode");
 
         let mut snapshot = state_snapshot(Vec::new());
         snapshot.call_to_play.display_name = "é".repeat(MAX_CALL_TO_PLAY_DISPLAY_NAME_CHARS);