Use Absolutize for files that do not exist

Canonicalize relies on the file existing to resolve the potential
traversal, which won't work for file upload in case the file name has a
traversal inside it.

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"

Cargo.toml

@@ -26,3 +26,4 @@ walkdir = "2"
 rayon = "1.3"
 notify = "4.0"
 tokio = "0.2"
+path-absolutize = "3.0.6"

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<PathBuf> {
         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<PathBuf> {
                 }
             })
             .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("../../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"));
     }
 
     #[test]

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);
+        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 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());
-
             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()