path validation

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?;
 

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<PathBuf> {
+    // 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<PathBuf> {
+    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<PathBuf> {
+    // 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<std::path::PathBuf> {
+        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<Self> {
+            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());
+    }
+}

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<u64>,
 ) -> 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?;