diff --git a/crates/lanspread-peer/src/lib.rs b/crates/lanspread-peer/src/lib.rs index eff6c2b..c5e702a 100644 --- a/crates/lanspread-peer/src/lib.rs +++ b/crates/lanspread-peer/src/lib.rs @@ -4,7 +4,7 @@ mod path_validation; mod peer; use std::{ - collections::HashMap, + collections::{HashMap, VecDeque}, net::SocketAddr, path::{Path, PathBuf}, sync::Arc, @@ -332,6 +332,7 @@ struct DownloadChunk { offset: u64, length: u64, retry_count: usize, + last_peer: Option, } #[derive(Debug, Default)] @@ -361,14 +362,12 @@ async fn prepare_game_storage( if let Some(parent) = validated_path.parent() { tokio::fs::create_dir_all(parent).await?; } - let file_size = desc.file_size().unwrap_or(0); - let file = OpenOptions::new() + OpenOptions::new() .create(true) .truncate(true) .write(true) .open(&validated_path) .await?; - file.set_len(file_size).await?; } } Ok(()) @@ -395,6 +394,7 @@ fn build_peer_plans( offset: 0, length: 0, retry_count: 0, + last_peer: Some(peer), }); continue; } @@ -409,6 +409,7 @@ fn build_peer_plans( offset, length, retry_count: 0, + last_peer: Some(peer), }); offset += length; } @@ -601,6 +602,7 @@ async fn download_from_peer( offset: 0, length: 0, // Indicates whole file retry_count: 0, + last_peer: Some(peer_addr), }; let result = download_whole_file(&mut conn, &base_dir, desc).await; @@ -658,6 +660,7 @@ async fn download_game_files( if chunk_result.chunk.retry_count < MAX_RETRY_COUNT { let mut retry_chunk = chunk_result.chunk; retry_chunk.retry_count += 1; + retry_chunk.last_peer = Some(chunk_result.peer_addr); failed_chunks.push(retry_chunk); } else { last_err = Some(eyre::eyre!( @@ -701,43 +704,123 @@ async fn download_game_files( Ok(()) } +fn select_retry_peer( + peers: &[SocketAddr], + last_peer: Option, + attempt_offset: usize, +) -> Option { + if peers.is_empty() { + return None; + } + + if peers.len() > 1 + && let Some(last) = last_peer + && let Some(pos) = peers.iter().position(|addr| *addr == last) + { + let next_index = (pos + 1 + attempt_offset) % peers.len(); + return Some(peers[next_index]); + } + + Some(peers[attempt_offset % peers.len()]) +} + +fn fallback_peer_addr(peers: &[SocketAddr], last_peer: Option) -> SocketAddr { + last_peer + .or_else(|| peers.first().copied()) + .unwrap_or_else(|| SocketAddr::from(([0, 0, 0, 0], 0))) +} + async fn retry_failed_chunks( failed_chunks: Vec, peers: &[SocketAddr], base_dir: &Path, game_id: &str, ) -> Vec { - let mut results = Vec::new(); + let mut exhausted = Vec::new(); + let mut queue: VecDeque = failed_chunks.into_iter().collect(); + + while let Some(mut chunk) = queue.pop_front() { + if chunk.retry_count >= MAX_RETRY_COUNT { + exhausted.push(ChunkDownloadResult { + chunk: chunk.clone(), + result: Err(eyre::eyre!( + "Retry budget exhausted for chunk: {}", + chunk.relative_path + )), + peer_addr: fallback_peer_addr(peers, chunk.last_peer), + }); + continue; + } + + let retry_offset = chunk.retry_count.saturating_sub(1); + let Some(peer_addr) = select_retry_peer(peers, chunk.last_peer, retry_offset) else { + exhausted.push(ChunkDownloadResult { + chunk: chunk.clone(), + result: Err(eyre::eyre!( + "No peers available to retry chunk: {}", + chunk.relative_path + )), + peer_addr: fallback_peer_addr(peers, chunk.last_peer), + }); + continue; + }; + + let mut attempt_chunk = chunk.clone(); + attempt_chunk.last_peer = Some(peer_addr); - // Redistribute failed chunks among available peers - let _retry_plans = build_peer_plans(peers, &[]); - for (i, chunk) in failed_chunks.into_iter().enumerate() { - let peer_addr = peers[i % peers.len()]; let plan = PeerDownloadPlan { - chunks: vec![chunk], + chunks: vec![attempt_chunk.clone()], whole_files: Vec::new(), }; match download_from_peer(peer_addr, game_id, plan, base_dir.to_path_buf()).await { - Ok(chunk_results) => results.extend(chunk_results), + Ok(results) => { + for result in results { + match result.result { + Ok(()) => {} + Err(e) => { + let mut retry_chunk = result.chunk.clone(); + retry_chunk.retry_count = chunk.retry_count + 1; + retry_chunk.last_peer = Some(result.peer_addr); + + if retry_chunk.retry_count >= MAX_RETRY_COUNT { + let context = format!( + "Retry budget exhausted for chunk: {}", + result.chunk.relative_path + ); + exhausted.push(ChunkDownloadResult { + chunk: retry_chunk, + result: Err(e.wrap_err(context)), + peer_addr: result.peer_addr, + }); + } else { + queue.push_back(retry_chunk); + } + } + } + } + } Err(e) => { - log::error!("Failed to retry chunk: {e}"); - // Add empty failure result - results.push(ChunkDownloadResult { - chunk: DownloadChunk { - relative_path: "unknown".to_string(), - offset: 0, - length: 0, - retry_count: MAX_RETRY_COUNT, - }, - result: Err(e), - peer_addr, - }); + chunk.retry_count += 1; + chunk.last_peer = Some(peer_addr); + + if chunk.retry_count >= MAX_RETRY_COUNT { + exhausted.push(ChunkDownloadResult { + chunk: chunk.clone(), + result: Err(e.wrap_err(format!( + "Retry budget exhausted for chunk after connection failure: {}", + chunk.relative_path + ))), + peer_addr: fallback_peer_addr(peers, chunk.last_peer), + }); + } else { + queue.push_back(chunk); + } } } } - results + exhausted } async fn load_local_game_db(game_dir: &str) -> eyre::Result { diff --git a/crates/lanspread-peer/src/path_validation.rs b/crates/lanspread-peer/src/path_validation.rs index 724a4c6..d7488ff 100644 --- a/crates/lanspread-peer/src/path_validation.rs +++ b/crates/lanspread-peer/src/path_validation.rs @@ -1,79 +1,90 @@ -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; + +use eyre::WrapErr; + +fn canonicalize_base_dir(base_dir: &Path) -> eyre::Result { + if !base_dir.is_absolute() { + eyre::bail!("Base directory must be absolute: {}", base_dir.display()); + } + + let canonical = std::fs::canonicalize(base_dir).unwrap_or_else(|_| base_dir.to_path_buf()); + Ok(canonical) +} + +fn sanitize_relative_path(relative_path: &str) -> eyre::Result { + if relative_path.is_empty() { + eyre::bail!("Relative path cannot be empty"); + } + + if relative_path.contains('\0') { + eyre::bail!("Path contains null byte"); + } + + // Normalise Windows separators so that component checks work uniformly. + let normalised = relative_path.replace('\\', "/"); + + if normalised.starts_with("//") { + eyre::bail!("UNC paths are not allowed: {relative_path}"); + } + + if normalised.contains(":/") { + eyre::bail!("Path contains drive letter: {relative_path}"); + } + + let path = PathBuf::from(&normalised); + if path.is_absolute() { + eyre::bail!("Path must be relative, not absolute: {relative_path}"); + } + + Ok(path) +} /// Validates and sanitizes a relative path to prevent directory traversal attacks. /// /// # Errors /// Returns an error if the path attempts to escape the base directory or contains invalid components. pub fn validate_relative_path(base_dir: &Path, relative_path: &str) -> eyre::Result { - // Create the full path by joining base directory with relative path - let full_path = base_dir.join(relative_path); + let canonical_base = canonicalize_base_dir(base_dir)?; + let relative = sanitize_relative_path(relative_path)?; - // Normalize the path to resolve any ".." or "." components - let canonical_path = match std::fs::canonicalize(&full_path) { - Ok(path) => path, - Err(_) => { - // If the file doesn't exist, we can't canonicalize it. - // Instead, we'll manually validate the path components. - validate_path_components(&full_path, base_dir)? - } - }; + let mut resolved = canonical_base.clone(); - // Get the canonical base directory - let canonical_base = match std::fs::canonicalize(base_dir) { - Ok(path) => path, - Err(_) => { - // If base directory doesn't exist, use its absolute form - match std::fs::canonicalize(base_dir) { - Ok(path) => path, - Err(_) => base_dir.to_path_buf(), - } - } - }; - - // Ensure the canonical path starts with the canonical base directory - if !canonical_path.starts_with(&canonical_base) { - eyre::bail!( - "Path validation failed: {} attempts to escape base directory", - relative_path - ); - } - - Ok(canonical_path) -} - -/// Validates path components without requiring the file to exist -fn validate_path_components(full_path: &Path, base_dir: &Path) -> eyre::Result { - let mut result = base_dir.to_path_buf(); - - for component in full_path.components() { + for component in relative.components() { match component { - std::path::Component::Prefix(_) => { - eyre::bail!("Path contains Windows prefix: {:?}", full_path); + Component::Prefix(_) | Component::RootDir => { + eyre::bail!("Path is not relative: {relative_path}"); } - std::path::Component::RootDir | std::path::Component::CurDir => { - // Skip root directory and current directory components + Component::ParentDir => { + eyre::bail!("Path attempts to escape base directory: {relative_path}"); } - std::path::Component::ParentDir => { - // Check if we can go up one level without escaping base_dir - if !result.pop() { - eyre::bail!( - "Path attempts to escape base directory with '..': {:?}", - full_path - ); + Component::CurDir => {} + Component::Normal(part) => { + resolved.push(part); + + if let Ok(metadata) = std::fs::symlink_metadata(&resolved) + && metadata.file_type().is_symlink() + { + let target = std::fs::canonicalize(&resolved).wrap_err_with(|| { + format!("Failed to canonicalize symlink {}", resolved.display()) + })?; + + if !target.starts_with(&canonical_base) { + eyre::bail!( + "Path validation failed: {relative_path} escapes the base directory" + ); + } + + resolved = target; } } - std::path::Component::Normal(name) => { - // Validate the component name - let name_str = name.to_string_lossy(); - if name_str.contains('\0') || name_str.contains('/') || name_str.contains('\\') { - eyre::bail!("Path component contains invalid characters: {}", name_str); - } - result.push(name); - } } } - Ok(result) + if !resolved.starts_with(&canonical_base) { + eyre::bail!("Path validation failed: {relative_path} escapes the base directory"); + } + + Ok(resolved) } /// Validates a relative path that will be used for accessing files within a game directory. @@ -82,24 +93,6 @@ fn validate_path_components(full_path: &Path, base_dir: &Path) -> eyre::Result

eyre::Result { - // First check if the relative path is actually relative - if Path::new(relative_path).is_absolute() { - eyre::bail!("Path must be relative, not absolute: {}", relative_path); - } - - // Check for obvious traversal attempts - if relative_path.contains("..") { - eyre::bail!("Path contains directory traversal: {}", relative_path); - } - - // For Windows paths - if relative_path.contains(':') - && (relative_path.contains(":\\") || relative_path.contains(":/")) - { - eyre::bail!("Path contains drive letter: {}", relative_path); - } - - // Use the general validation function validate_relative_path(game_dir, relative_path) } @@ -109,7 +102,15 @@ mod tests { fn create_temp_dir() -> std::io::Result { let mut dir = std::env::temp_dir(); - dir.push(format!("lanspread_test_{}", std::process::id())); + let unique = format!( + "lanspread_test_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() + ); + dir.push(unique); std::fs::create_dir_all(&dir)?; Ok(dir) } @@ -143,6 +144,13 @@ mod tests { assert!(validate_game_file_path(base, "subdir/file.txt").is_ok()); assert!(validate_game_file_path(base, "subdir/deep/nested/file.txt").is_ok()); assert!(validate_game_file_path(base, "file with spaces.txt").is_ok()); + assert!(validate_game_file_path(base, "./dot/./path.txt").is_ok()); + + let windows_style = validate_game_file_path(base, "mix\\windows\\path.txt").unwrap(); + assert_eq!( + windows_style, + base.join("mix").join("windows").join("path.txt") + ); } #[test] @@ -156,6 +164,29 @@ mod tests { assert!(validate_game_file_path(base, "../../etc/passwd").is_err()); } + #[test] + fn test_double_dot_in_filename_allowed() { + let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); + let base = temp_dir.path(); + + assert!(validate_game_file_path(base, "data/file..txt").is_ok()); + } + + #[test] + fn test_missing_file_stays_within_base() { + let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); + let base = temp_dir.path(); + + let resolved = validate_game_file_path(base, "new_dir/new_file.bin").unwrap(); + assert_eq!(resolved, base.join("new_dir").join("new_file.bin")); + } + + #[test] + fn test_relative_base_dir_rejected() { + let relative_base = std::path::Path::new("relative/base"); + assert!(validate_game_file_path(relative_base, "file.bin").is_err()); + } + #[test] fn test_absolute_paths() { let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); @@ -178,4 +209,20 @@ mod tests { assert!(validate_game_file_path(base, "D:/evil.txt").is_err()); assert!(validate_game_file_path(base, "\\\\server\\share\\file.txt").is_err()); } + + #[cfg(unix)] + #[test] + fn test_symlink_escape_rejected() { + use std::os::unix::fs::symlink; + + let base_dir = TempDir::new().expect("Failed to create base temp dir"); + let outside_dir = TempDir::new().expect("Failed to create outside temp dir"); + + let base = base_dir.path(); + let outside = outside_dir.path(); + + symlink(outside, base.join("link")).expect("Failed to create symlink"); + + assert!(validate_game_file_path(base, "link/escape.txt").is_err()); + } }