diff --git a/Cargo.lock b/Cargo.lock index e05df4c..f19b113 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1133,6 +1133,7 @@ dependencies = [ "image", "jsonwebtoken", "notify", + "path-absolutize", "rayon", "serde", "serde_json", @@ -1596,6 +1597,24 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "path-absolutize" +version = "3.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a6ab2aaa5faefed84db46e4398eab15fa51325606462b5da8b0e230af3ac59a" +dependencies = [ + "path-dedot", +] + +[[package]] +name = "path-dedot" +version = "3.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "658c6e985fce9c25289fe7c86c08a3cbe82c19a3cd5b3bc5945c8c632552e460" +dependencies = [ + "once_cell", +] + [[package]] name = "pem" version = "0.8.1" diff --git a/Cargo.toml b/Cargo.toml index 3298ec2..9bf8492 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,3 +26,4 @@ walkdir = "2" rayon = "1.3" notify = "4.0" tokio = "0.2" +path-absolutize = "3.0.6" diff --git a/src/files.rs b/src/files.rs index 9ef7be4..cff8813 100644 --- a/src/files.rs +++ b/src/files.rs @@ -1,3 +1,4 @@ +use path_absolutize::*; use std::ffi::OsStr; use std::fs::read_dir; use std::io; @@ -47,10 +48,10 @@ fn is_valid_full_path(base: &Path, path: &str) -> Option { let mut full_path = PathBuf::from(base); full_path.push(&path); full_path - .canonicalize() + .absolutize() .and_then(|p| { if p.starts_with(base) { - Ok(p) + Ok(p.into_owned()) } else { Err(io::Error::new( io::ErrorKind::Other, @@ -59,9 +60,9 @@ fn is_valid_full_path(base: &Path, path: &str) -> Option { } }) .ok() - } else if let Ok(path) = path.canonicalize().and_then(|path| { + } else if let Ok(path) = path.absolutize().and_then(|path| { if path.starts_with(base) { - Ok(path) + Ok(path.into_owned()) } else { Err(io::Error::new( io::ErrorKind::Other, @@ -88,9 +89,7 @@ mod tests { assert_eq!(None, is_valid_path("fake/../../../")); assert_eq!(None, is_valid_path("../../../etc/passwd")); assert_eq!(None, is_valid_path("..//etc/passwd")); - assert_eq!(None, is_valid_path("..%2f..%2f/etc/passwd")); - assert_eq!(None, is_valid_path("%2e%2e%2f%2e%2e%2f/etc/passwd")); - assert_eq!(None, is_valid_path("\\\\attacker.com\\shared\\mal.php")); + assert_eq!(None, is_valid_path("../../etc/passwd")); } #[test] diff --git a/src/main.rs b/src/main.rs index 072c4b0..cadb131 100644 --- a/src/main.rs +++ b/src/main.rs @@ -153,13 +153,8 @@ async fn upload_image(_: Claims, mut payload: mp::Multipart) -> impl Responder { let path = file_path.unwrap_or_else(|| dotenv::var("BASE_PATH").unwrap()); if !file_content.is_empty() { - let full_path = PathBuf::from(&path); - - if let Some(mut full_path) = is_valid_path(full_path.to_str().unwrap_or("")) { - // TODO: Validate this file_name as is subject to path traversals which could lead to - // writing outside the base dir. - full_path = full_path.join(file_name.unwrap()); - + let full_path = PathBuf::from(&path).join(file_name.unwrap()); + if let Some(full_path) = is_valid_path(full_path.to_str().unwrap_or("")) { if !full_path.is_file() { let mut file = File::create(full_path).unwrap(); file.write_all(&file_content).unwrap(); @@ -206,8 +201,7 @@ async fn stream_video( // Extract video playlist dir to dotenv if !playlist.starts_with("tmp") || playlist.contains("..") { HttpResponse::NotFound().finish() - } - else if let Ok(file) = NamedFile::open(playlist) { + } else if let Ok(file) = NamedFile::open(playlist) { file.into_response(&request).unwrap() } else { HttpResponse::NotFound().finish()