From 7de373afb698b9c6a7ecb5d3a1fa7cee282f2905 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 12 Sep 2026 12:19:50 +0200 Subject: [PATCH] fix(peer): sanitize legacy library index migration The selected game root could supply an unbounded legacy library index whose revision was copied verbatim into app-owned state. A revision of `u64::MAX` made every later checked revision advance fail even after the original root was removed. Read only a bounded regular, non-link file whose identity stays stable across the read. Deserialize it before publication, preserve its cached game data, and reset its stale revision authority to zero. Existing app-owned state still wins without reading the legacy source, and invalid input remains in place for recovery. Test Plan: - `just test` -- passed outside the sandbox; 494 peer tests and all workspace targets passed. - The initial sandboxed `just test` reached an unrelated Unix-socket permission denial, then passed unchanged with the required socket permission. - `git diff --cached --check` -- passed. --- crates/lanspread-peer/src/local_games.rs | 31 +++++ crates/lanspread-peer/src/migration.rs | 153 +++++++++++++++++++++-- 2 files changed, 174 insertions(+), 10 deletions(-) diff --git a/crates/lanspread-peer/src/local_games.rs b/crates/lanspread-peer/src/local_games.rs index db37f7a..c6e42a9 100644 --- a/crates/lanspread-peer/src/local_games.rs +++ b/crates/lanspread-peer/src/local_games.rs @@ -191,6 +191,14 @@ struct LibraryIndex { games: HashMap, } +/// Converts a legacy selected-root cache into app-owned state without carrying +/// its stale revision authority across the trust boundary. +pub(crate) fn normalize_migrated_library_index(bytes: &[u8]) -> serde_json::Result> { + let mut index: LibraryIndex = serde_json::from_slice(bytes)?; + index.revision = 0; + serde_json::to_vec(&index) +} + #[derive(Debug, Clone, Serialize, Deserialize)] struct GameIndexEntry { summary: LocalGameSummary, @@ -1043,6 +1051,29 @@ mod tests { } } + #[test] + fn migrated_library_index_preserves_games_but_resets_revision_authority() { + let original = test_library_index(u64::MAX, "game", 77); + let bytes = serde_json::to_vec(&original).expect("legacy index should serialize"); + + let normalized = + normalize_migrated_library_index(&bytes).expect("legacy index should normalize"); + let migrated: LibraryIndex = + serde_json::from_slice(&normalized).expect("normalized index should deserialize"); + + assert_eq!(migrated.revision, 0); + assert_eq!(migrated.games.len(), 1); + assert_eq!( + migrated + .games + .get("game") + .expect("game should be preserved") + .summary + .size, + 77, + ); + } + #[test] fn legacy_fingerprint_defaults_download_recovery_to_false() { let fingerprint: GameFingerprint = serde_json::from_value(serde_json::json!({ diff --git a/crates/lanspread-peer/src/migration.rs b/crates/lanspread-peer/src/migration.rs index 7b5efe0..68a6999 100644 --- a/crates/lanspread-peer/src/migration.rs +++ b/crates/lanspread-peer/src/migration.rs @@ -1,6 +1,6 @@ use std::{ fs, - io::{ErrorKind, Write as _}, + io::{ErrorKind, Read as _, Write as _}, path::{Path, PathBuf}, sync::atomic::{AtomicUsize, Ordering}, thread, @@ -16,12 +16,13 @@ use crate::{ LEGACY_SOFTLAN_INSTALL_MARKER, is_ignored_games_root_name, }, - local_games::legacy_library_index_path, + local_games::{legacy_library_index_path, normalize_migrated_library_index}, scoped_blocking::scoped_blocking, state_paths::{local_library_index_path, setup_done_path}, }; const MIGRATION_CONCURRENCY: usize = 16; +const MAX_LEGACY_LIBRARY_INDEX_BYTES: u64 = 128 * 1024 * 1024; #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Serialize)] pub struct MigrationReport { @@ -140,7 +141,7 @@ fn migrate_library_index(game_dir: &Path, state_dir: &Path) -> MigrationReport { let legacy_path = legacy_library_index_path(game_dir); let target_path = local_library_index_path(state_dir); - match scoped_blocking(|| migrate_raw_file(&legacy_path, &target_path)) { + match scoped_blocking(|| migrate_library_index_file(&legacy_path, &target_path)) { Ok(MigrationOutcome::Migrated) => { report.library_index_migrated = true; report.legacy_files_deleted += 1; @@ -291,7 +292,10 @@ enum MigrationOutcome { Migrated, } -fn migrate_raw_file(legacy_path: &Path, target_path: &Path) -> std::io::Result { +fn migrate_library_index_file( + legacy_path: &Path, + target_path: &Path, +) -> std::io::Result { if !path_exists(legacy_path)? { return Ok(MigrationOutcome::SourceMissing); } @@ -301,12 +305,91 @@ fn migrate_raw_file(legacy_path: &Path, target_path: &Path) -> std::io::Result std::io::Result> { + let before = fs::symlink_metadata(path)?; + if !before.is_file() || is_link_or_reparse(&before) || before.len() > limit { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "legacy library index is not a bounded regular file", + )); + } + + let mut file = fs::File::open(path)?; + let opened = file.metadata()?; + if !opened.is_file() || !same_file(&before, &opened) || opened.len() > limit { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "legacy library index changed while opening", + )); + } + + let capacity = usize::try_from(opened.len().min(limit)) + .map_err(|error| std::io::Error::new(ErrorKind::InvalidData, error))?; + let mut bytes = Vec::with_capacity(capacity); + std::io::Read::by_ref(&mut file) + .take(limit + 1) + .read_to_end(&mut bytes)?; + if u64::try_from(bytes.len()).map_or(true, |length| length > limit) { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "legacy library index exceeds its byte limit", + )); + } + + let after = file.metadata()?; + if !after.is_file() || !same_file(&before, &after) || after.len() != opened.len() { + return Err(std::io::Error::new( + ErrorKind::InvalidData, + "legacy library index changed while reading", + )); + } + Ok(bytes) +} + +#[cfg(unix)] +fn same_file(before: &fs::Metadata, after: &fs::Metadata) -> bool { + use std::os::unix::fs::MetadataExt as _; + + before.dev() == after.dev() && before.ino() == after.ino() +} + +#[cfg(windows)] +fn same_file(before: &fs::Metadata, after: &fs::Metadata) -> bool { + use std::os::windows::fs::MetadataExt as _; + + before.volume_serial_number().is_some() + && before.volume_serial_number() == after.volume_serial_number() + && before.file_index().is_some() + && before.file_index() == after.file_index() +} + +#[cfg(not(any(unix, windows)))] +const fn same_file(_before: &fs::Metadata, _after: &fs::Metadata) -> bool { + false +} + +#[cfg(windows)] +fn is_link_or_reparse(metadata: &fs::Metadata) -> bool { + use std::os::windows::fs::MetadataExt as _; + + const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x400; + metadata.file_type().is_symlink() + || metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 +} + +#[cfg(not(windows))] +fn is_link_or_reparse(metadata: &fs::Metadata) -> bool { + metadata.file_type().is_symlink() +} + fn migrate_empty_marker( legacy_path: &Path, target_path: &Path, @@ -493,21 +576,71 @@ mod tests { let target_path = local_library_index_path(state.path()); let legacy_tmp_path = library_index_tmp_path(&legacy_path); - write_file(&legacy_path, br#"{"revision":7,"games":{}}"#); + write_file( + &legacy_path, + br#"{"revision":18446744073709551615,"games":{}}"#, + ); write_file(&legacy_tmp_path, b"tmp"); let report = migrate_legacy_state(games.path(), state.path()).await; assert!(report.library_index_migrated); - assert_eq!( - std::fs::read_to_string(&target_path).expect("index should migrate"), - r#"{"revision":7,"games":{}}"# - ); + let migrated: serde_json::Value = + serde_json::from_slice(&std::fs::read(&target_path).expect("index should migrate")) + .expect("migrated index should remain valid JSON"); + assert_eq!(migrated["revision"], 0); + assert_eq!(migrated["games"], serde_json::json!({})); assert!(!legacy_path.exists()); assert!(!legacy_tmp_path.exists()); assert!(!games.path().join(LEGACY_LIBRARY_INDEX_DIR).exists()); } + #[tokio::test] + async fn oversized_legacy_library_index_is_left_unmigrated() { + let games = TempDir::new("lanspread-migration-games"); + let state = TempDir::new("lanspread-migration-state"); + let legacy_path = legacy_library_index_path(games.path()); + let target_path = local_library_index_path(state.path()); + std::fs::create_dir_all(legacy_path.parent().expect("legacy index has a parent")) + .expect("legacy parent should be created"); + std::fs::File::create(&legacy_path) + .expect("legacy index should be created") + .set_len(MAX_LEGACY_LIBRARY_INDEX_BYTES + 1) + .expect("legacy index should become oversized"); + + let report = migrate_legacy_state(games.path(), state.path()).await; + + assert_eq!(report.failures, 1); + assert!(!report.library_index_migrated); + assert!(legacy_path.exists()); + assert!(!target_path.exists()); + } + + #[tokio::test] + async fn existing_app_index_wins_without_reading_oversized_legacy_bytes() { + let games = TempDir::new("lanspread-migration-games"); + let state = TempDir::new("lanspread-migration-state"); + let legacy_path = legacy_library_index_path(games.path()); + let target_path = local_library_index_path(state.path()); + std::fs::create_dir_all(legacy_path.parent().expect("legacy index has a parent")) + .expect("legacy parent should be created"); + std::fs::File::create(&legacy_path) + .expect("legacy index should be created") + .set_len(MAX_LEGACY_LIBRARY_INDEX_BYTES + 1) + .expect("legacy index should become oversized"); + write_file(&target_path, br#"{"revision":9,"games":{}}"#); + + let report = migrate_legacy_state(games.path(), state.path()).await; + + assert_eq!(report.failures, 0); + assert!(!report.library_index_migrated); + assert!(!legacy_path.exists()); + assert_eq!( + std::fs::read_to_string(target_path).expect("existing index should remain"), + r#"{"revision":9,"games":{}}"#, + ); + } + #[tokio::test] async fn legacy_install_intent_is_rejected_without_deletion() { let games = TempDir::new("lanspread-migration-games");