From 607d58df84337139014928fa99a0970767a9aca0 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 28 Apr 2024 14:58:55 +0100 Subject: [PATCH] feat(fs): properly validates paths are within base directory --- Cargo.toml | 1 + src/fs/real.rs | 17 +++++++++++++++-- src/tests/fs.rs | 41 +++++++++++++++++++++++++++-------------- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dd177dc..feb7876 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ thiserror = "1.0" # fs tempfile = "3.10" +path-clean = "1.0" # error handling derive_more = { version = "1.0.0-beta.6", features = ["from", "display"] } diff --git a/src/fs/real.rs b/src/fs/real.rs index d91fc9d..a975e2f 100644 --- a/src/fs/real.rs +++ b/src/fs/real.rs @@ -57,12 +57,25 @@ impl super::FileSystemLike for RealFileSystem { impl RealFileSystem { fn validate(&self, path: &Path) -> super::Result<()> { - if !path.starts_with(&self.base) { + let canon = self.clean_path(path)?; + if !canon.starts_with(&self.base) { return Err(super::Error::PathTraversal { base: self.base.clone(), - path: path.to_path_buf(), + path: canon.to_path_buf(), }); } Ok(()) } + + fn clean_path(&self, path: &Path) -> super::Result { + // let path = path.as_ref(); + use path_clean::PathClean; + let abs_path = if path.is_absolute() { + path.to_path_buf() + } else { + std::env::current_dir()?.join(path) + } + .clean(); + Ok(abs_path) + } } diff --git a/src/tests/fs.rs b/src/tests/fs.rs index 5123ab5..aa358b8 100644 --- a/src/tests/fs.rs +++ b/src/tests/fs.rs @@ -1,22 +1,35 @@ -use std::path::PathBuf; +use assert2::let_assert; use crate::fs; -type TestResult = Result<(), crate::fs::Error>; +type TestResult = Result<(), fs::Error>; + +#[test] +fn path_of_validate_fails_path_traversal() -> TestResult { + let fs = fs::temp()?; + + let_assert!(Err(fs::Error::PathTraversal { base, path: _path }) = fs.path_of("..".into())); + assert_eq!(base, fs.base()); + + Ok(()) +} #[test] fn write_read_file_exists() -> TestResult { let fs = fs::temp()?; - let pathbuf: PathBuf = fs.path_of("foo".into())?; + let pathbuf = fs.base().join("foo"); - fs.file_write(&pathbuf, "content")?; - let c = fs.file_read_to_string(&pathbuf)?; + let_assert!(Ok(_) = fs.file_write(&pathbuf, "content")); + let_assert!( + Ok(c) = fs.file_read_to_string(&pathbuf), + "file_read_to_string" + ); assert_eq!(c, "content"); - let exists = fs.path_exists(&pathbuf)?; + let_assert!(Ok(exists) = fs.path_exists(&pathbuf)); assert!(exists); - let is_file = fs.path_is_file(&pathbuf)?; + let_assert!(Ok(is_file) = fs.path_is_file(&pathbuf)); assert!(is_file); Ok(()) @@ -25,14 +38,14 @@ fn write_read_file_exists() -> TestResult { #[test] fn create_dir_should_create_a_dir() -> TestResult { let fs = fs::temp()?; - let pathbuf = fs.path_of("subdir".into())?; + let pathbuf = fs.base().join("subdir"); - fs.dir_create(&pathbuf)?; + let_assert!(Ok(_) = fs.dir_create(&pathbuf)); - let exists = fs.path_exists(&pathbuf)?; + let_assert!(Ok(exists) = fs.path_exists(&pathbuf)); assert!(exists); - let is_dir = fs.path_is_dir(&pathbuf)?; + let_assert!(Ok(is_dir) = fs.path_is_dir(&pathbuf)); assert!(is_dir); Ok(()) @@ -43,12 +56,12 @@ fn create_dir_all_should_create_a_dir() -> TestResult { let fs = fs::temp()?; let pathbuf = fs.base().join("subdir").join("child"); - fs.dir_create_all(&pathbuf)?; + let_assert!(Ok(_) = fs.dir_create_all(&pathbuf)); - let exists = fs.path_exists(&pathbuf)?; + let_assert!(Ok(exists) = fs.path_exists(&pathbuf)); assert!(exists, "path exists"); - let is_dir = fs.path_is_dir(&pathbuf)?; + let_assert!(Ok(is_dir) = fs.path_is_dir(&pathbuf)); assert!(is_dir, "path is a directory"); Ok(())