diff --git a/crates/lanspread-peer/src/lib.rs b/crates/lanspread-peer/src/lib.rs index b196962..eff6c2b 100644 --- a/crates/lanspread-peer/src/lib.rs +++ b/crates/lanspread-peer/src/lib.rs @@ -1,5 +1,6 @@ #![allow(clippy::missing_errors_doc)] +mod path_validation; mod peer; use std::{ @@ -32,7 +33,10 @@ use tokio::{ }; use uuid::Uuid; -use crate::peer::{send_game_file_chunk, send_game_file_data}; +use crate::{ + path_validation::validate_game_file_path, + peer::{send_game_file_chunk, send_game_file_data}, +}; /// Initialize and start the peer system /// This function replaces the main.rs entry point and allows the peer to be started from other crates @@ -348,11 +352,13 @@ async fn prepare_game_storage( file_descs: &[GameFileDescription], ) -> eyre::Result<()> { for desc in file_descs { - let path = games_folder.join(&desc.relative_path); + // Validate the path to prevent directory traversal + let validated_path = validate_game_file_path(games_folder, &desc.relative_path)?; + if desc.is_dir { - tokio::fs::create_dir_all(&path).await?; + tokio::fs::create_dir_all(&validated_path).await?; } else { - if let Some(parent) = path.parent() { + if let Some(parent) = validated_path.parent() { tokio::fs::create_dir_all(parent).await?; } let file_size = desc.file_size().unwrap_or(0); @@ -360,7 +366,7 @@ async fn prepare_game_storage( .create(true) .truncate(true) .write(true) - .open(&path) + .open(&validated_path) .await?; file.set_len(file_size).await?; } @@ -450,12 +456,13 @@ async fn download_chunk( tx.close().await?; - let path = base_dir.join(&chunk.relative_path); + // Validate the path to prevent directory traversal + let validated_path = validate_game_file_path(base_dir, &chunk.relative_path)?; let mut file = OpenOptions::new() .create(true) .write(true) .truncate(false) - .open(&path) + .open(&validated_path) .await?; if chunk.length == 0 && chunk.offset == 0 { // fallback-to-whole-file path replaces any existing partial data @@ -493,7 +500,7 @@ async fn download_chunk( file.flush().await?; // Verify file integrity by checking the file size - verify_chunk_integrity(&path, chunk.offset, chunk.length).await?; + verify_chunk_integrity(&validated_path, chunk.offset, chunk.length).await?; Ok(()) } @@ -534,12 +541,13 @@ async fn download_whole_file( .await?; tx.close().await?; - let path = base_dir.join(&desc.relative_path); + // Validate the path to prevent directory traversal + let validated_path = validate_game_file_path(base_dir, &desc.relative_path)?; let mut file = OpenOptions::new() .create(true) .truncate(true) .write(true) - .open(&path) + .open(&validated_path) .await?; file.seek(std::io::SeekFrom::Start(0)).await?; diff --git a/crates/lanspread-peer/src/path_validation.rs b/crates/lanspread-peer/src/path_validation.rs new file mode 100644 index 0000000..724a4c6 --- /dev/null +++ b/crates/lanspread-peer/src/path_validation.rs @@ -0,0 +1,181 @@ +use std::path::{Path, PathBuf}; + +/// 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); + + // 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)? + } + }; + + // 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() { + match component { + std::path::Component::Prefix(_) => { + eyre::bail!("Path contains Windows prefix: {:?}", full_path); + } + std::path::Component::RootDir | std::path::Component::CurDir => { + // Skip root directory and current directory components + } + 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 + ); + } + } + 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) +} + +/// Validates a relative path that will be used for accessing files within a game directory. +/// This is a stricter validation that ensures the path is relative and doesn't escape. +/// +/// # Errors +/// Returns an error if the path is absolute or attempts directory traversal. +pub fn validate_game_file_path(game_dir: &Path, relative_path: &str) -> 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) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn create_temp_dir() -> std::io::Result { + let mut dir = std::env::temp_dir(); + dir.push(format!("lanspread_test_{}", std::process::id())); + std::fs::create_dir_all(&dir)?; + Ok(dir) + } + + struct TempDir(std::path::PathBuf); + + impl TempDir { + fn new() -> std::io::Result { + let path = create_temp_dir()?; + Ok(TempDir(path)) + } + + fn path(&self) -> &std::path::Path { + &self.0 + } + } + + impl Drop for TempDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn test_valid_paths() { + let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); + let base = temp_dir.path(); + + // Valid relative paths + assert!(validate_game_file_path(base, "file.txt").is_ok()); + 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()); + } + + #[test] + fn test_traversal_attempts() { + let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); + let base = temp_dir.path(); + + // These should all fail + assert!(validate_game_file_path(base, "../outside.txt").is_err()); + assert!(validate_game_file_path(base, "subdir/../../outside.txt").is_err()); + assert!(validate_game_file_path(base, "../../etc/passwd").is_err()); + } + + #[test] + fn test_absolute_paths() { + let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); + let base = temp_dir.path(); + + // Absolute paths should fail + assert!(validate_game_file_path(base, "/etc/passwd").is_err()); + if cfg!(windows) { + assert!(validate_game_file_path(base, "C:\\Windows\\System32\\cmd.exe").is_err()); + } + } + + #[test] + fn test_windows_specific() { + let temp_dir = TempDir::new().expect("Failed to create temp dir for test"); + let base = temp_dir.path(); + + // Windows-specific paths that should fail + assert!(validate_game_file_path(base, "C:\\evil.txt").is_err()); + assert!(validate_game_file_path(base, "D:/evil.txt").is_err()); + assert!(validate_game_file_path(base, "\\\\server\\share\\file.txt").is_err()); + } +} diff --git a/crates/lanspread-peer/src/peer.rs b/crates/lanspread-peer/src/peer.rs index 1d6d396..fd0e235 100644 --- a/crates/lanspread-peer/src/peer.rs +++ b/crates/lanspread-peer/src/peer.rs @@ -9,6 +9,8 @@ use tokio::{ time::Instant, }; +use crate::path_validation::validate_game_file_path; + async fn stream_file_bytes( tx: &mut SendStream, base_dir: &Path, @@ -17,13 +19,15 @@ async fn stream_file_bytes( length: Option, ) -> eyre::Result<()> { let remote_addr = maybe_addr!(tx.connection().remote_addr()); - let game_file = base_dir.join(relative_path); + + // Validate the path to prevent directory traversal + let validated_path = validate_game_file_path(base_dir, relative_path)?; log::debug!( "{remote_addr} streaming file bytes for peer: {}, offset: {offset}, length: {length:?}", - game_file.display() + validated_path.display() ); - let mut file = tokio::fs::File::open(&game_file).await?; + let mut file = tokio::fs::File::open(&validated_path).await?; if offset > 0 { file.seek(std::io::SeekFrom::Start(offset)).await?; } @@ -59,7 +63,7 @@ async fn stream_file_bytes( let mb_per_s = (diff_bytes as f64) / (elapsed.as_secs_f64() * 1_000_000.0); log::debug!( "{remote_addr} sending file data: {}, MB/s: {mb_per_s:.2}", - game_file.display() + validated_path.display() ); last_total_bytes = total_bytes; timestamp = Instant::now(); @@ -69,7 +73,7 @@ async fn stream_file_bytes( log::debug!( "{remote_addr} finished streaming file bytes: {}, total_bytes: {total_bytes}", - game_file.display() + validated_path.display() ); tx.close().await?;