diff --git a/Cargo.lock b/Cargo.lock index a247391..acc3ff6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3193,6 +3193,7 @@ dependencies = [ "linkme", "matchit 0.7.3", "multer", + "percent-encoding", "pin-project-lite", "prometheus", "proptest", diff --git a/crates/rustapi-core/Cargo.toml b/crates/rustapi-core/Cargo.toml index 9fc67f0..9810276 100644 --- a/crates/rustapi-core/Cargo.toml +++ b/crates/rustapi-core/Cargo.toml @@ -10,6 +10,7 @@ repository.workspace = true homepage.workspace = true [dependencies] +percent-encoding = "2.3" # Async tokio = { workspace = true, features = ["rt", "net", "time", "fs", "macros", "io-util"] } futures-util = { workspace = true } diff --git a/crates/rustapi-core/src/static_files.rs b/crates/rustapi-core/src/static_files.rs index 2f3e046..1ea50a4 100644 --- a/crates/rustapi-core/src/static_files.rs +++ b/crates/rustapi-core/src/static_files.rs @@ -253,8 +253,13 @@ impl StaticFile { relative_path: &str, config: &StaticFileConfig, ) -> Result { - // Sanitize path to prevent directory traversal - let clean_path = sanitize_path(relative_path); + // Percent-decode the path first + let decoded_path = percent_encoding::percent_decode_str(relative_path) + .decode_utf8() + .unwrap_or(std::borrow::Cow::Borrowed(relative_path)); + + // Sanitize path to prevent basic directory traversal + let clean_path = sanitize_path(&decoded_path); let file_path = config.root.join(&clean_path); // Check if it's a directory @@ -282,6 +287,21 @@ impl StaticFile { /// Serve a specific file async fn serve_file(path: &Path, config: &StaticFileConfig) -> Result { + // Security check: ensure the resolved path is within the root directory + let canonical_root = match tokio::fs::canonicalize(&config.root).await { + Ok(root) => root, + Err(_) => return Err(ApiError::internal("Static file root directory not found")), + }; + + let canonical_file = match tokio::fs::canonicalize(path).await { + Ok(file) => file, + Err(_) => return Err(ApiError::not_found("File not found")), + }; + + if !canonical_file.starts_with(&canonical_root) { + return Err(ApiError::not_found("File not found")); + } + // Check if file exists let metadata = fs::metadata(path) .await diff --git a/crates/rustapi-core/tests/static_files_security_test.rs b/crates/rustapi-core/tests/static_files_security_test.rs new file mode 100644 index 0000000..1774325 --- /dev/null +++ b/crates/rustapi-core/tests/static_files_security_test.rs @@ -0,0 +1,53 @@ +use rustapi_core::static_files::serve_dir; +use rustapi_core::static_files::StaticFile; +use std::fs::File; + +#[tokio::test] +async fn test_directory_traversal_blocked() { + let config = serve_dir("/static", "./"); // root is crates/rustapi-core + + // We try to access something outside the root + let relative_path = "../../etc/passwd"; + let res = StaticFile::serve(relative_path, &config).await; + assert!(res.is_err(), "Standard traversal should be blocked"); + + // Percent encoded payload + let relative_path_encoded = "..%2F..%2Fetc%2Fpasswd"; + let res_encoded = StaticFile::serve(relative_path_encoded, &config).await; + assert!(res_encoded.is_err(), "Encoded traversal should be blocked"); + + // Double encoded + let relative_path_double = "%2e%2e%2f%2e%2e%2fetc%2fpasswd"; + let res_double = StaticFile::serve(relative_path_double, &config).await; + assert!( + res_double.is_err(), + "Double encoded traversal should be blocked" + ); +} + +#[tokio::test] +async fn test_valid_file_served() { + let config = serve_dir("/static", "./"); + + // Valid file + let relative_path = "src/lib.rs"; + let res = StaticFile::serve(relative_path, &config).await; + assert!(res.is_ok(), "Valid file should be served"); +} + +#[tokio::test] +async fn test_valid_file_with_spaces_served() { + let _ = std::fs::create_dir_all("./test_dir"); + let _ = File::create("./test_dir/file with spaces.txt"); + + let config = serve_dir("/static", "./test_dir"); + + let relative_path = "file%20with%20spaces.txt"; + let res = StaticFile::serve(relative_path, &config).await; + assert!( + res.is_ok(), + "File with percent-encoded spaces should be served" + ); + + let _ = std::fs::remove_dir_all("./test_dir"); +}