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
This commit is contained in:
2026-09-02 22:27:39 +02:00
parent e938767d66
commit c6d159d5f4
+47
View File
@@ -635,6 +635,9 @@ pub enum ControlValidationError {
field: &'static str, field: &'static str,
maximum: usize, maximum: usize,
}, },
InvalidPathComponent {
field: &'static str,
},
DuplicateGameId, DuplicateGameId,
UnsortedGameIds, UnsortedGameIds,
} }
@@ -657,6 +660,9 @@ impl fmt::Display for ControlValidationError {
Self::TooManyItems { field, maximum } => { Self::TooManyItems { field, maximum } => {
write!(formatter, "{field} exceeds its {maximum}-item limit") 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::DuplicateGameId => formatter.write_str("library contains a duplicate game ID"),
Self::UnsortedGameIds => { Self::UnsortedGameIds => {
formatter.write_str("library game IDs are not strictly sorted") 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> { fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
if game_id.trim().is_empty() { if game_id.trim().is_empty() {
return Err(ControlValidationError::EmptyField { field: "game ID" }); 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, 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(()) Ok(())
} }
@@ -1530,6 +1547,36 @@ mod tests {
.encode() .encode()
.is_err() .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()); let mut snapshot = state_snapshot(Vec::new());
snapshot.call_to_play.display_name = "é".repeat(MAX_CALL_TO_PLAY_DISPLAY_NAME_CHARS); snapshot.call_to_play.display_name = "é".repeat(MAX_CALL_TO_PLAY_DISPLAY_NAME_CHARS);