Do path joins more safely

src/paste.rs

@@ -63,7 +63,7 @@ impl PasteType {
         if dir.is_empty() {
             path.to_path_buf()
         } else {
-            path.join(dir)
+            util::safe_path_join(path, Path::new(&dir)).unwrap()
         }
     }
 
@@ -122,10 +122,12 @@ impl Paste {
         if let Some(handle_spaces_config) = config.server.handle_spaces {
             file_name = handle_spaces_config.process_filename(&file_name);
         }
-        let mut path = self
-            .type_
-            .get_path(&config.server.upload_path)
-            .join(&file_name);
+        let mut path =
+            util::safe_path_join(self.type_.get_path(&config.server.upload_path), &file_name)
+                .ok_or(IoError::new(
+                    IoErrorKind::Other,
+                    String::from("invalid filename"),
+                ))?;
         let mut parts: Vec<&str> = file_name.split('.').collect();
         let mut dotfile = false;
         let mut lower_bound = 1;
@@ -261,10 +263,12 @@ impl Paste {
                 file_name = random_text;
             }
         }
-        let mut path = self
-            .type_
-            .get_path(&config.server.upload_path)
-            .join(&file_name);
+        let mut path =
+            util::safe_path_join(self.type_.get_path(&config.server.upload_path), &file_name)
+                .ok_or(IoError::new(
+                    IoErrorKind::Other,
+                    String::from("Invalid filename"),
+                ))?;
         if let Some(timestamp) = expiry_date {
             path.set_file_name(format!("{file_name}.{timestamp}"));
         }

src/server.rs

@@ -4,7 +4,7 @@ use crate::file::Directory;
 use crate::header::{self, ContentDisposition};
 use crate::mime as mime_util;
 use crate::paste::{Paste, PasteType};
-use crate::util;
+use crate::util::{self, safe_path_join};
 use actix_files::NamedFile;
 use actix_multipart::Multipart;
 use actix_web::http::StatusCode;
@@ -89,12 +89,14 @@ async fn serve(
     let config = config
         .read()
         .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?;
-    let path = config.server.upload_path.join(&*file);
+    let path = safe_path_join(&config.server.upload_path, &*file)
+        .ok_or(error::ErrorInternalServerError("Invalid filename"))?;
     let mut path = util::glob_match_file(path)?;
     let mut paste_type = PasteType::File;
     if !path.exists() || path.is_dir() {
         for type_ in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] {
-            let alt_path = type_.get_path(&config.server.upload_path).join(&*file);
+            let alt_path = safe_path_join(type_.get_path(&config.server.upload_path), &*file)
+                .ok_or(error::ErrorInternalServerError("Invalid filename"))?;
             let alt_path = util::glob_match_file(alt_path)?;
             if alt_path.exists()
                 || path.file_name().and_then(|v| v.to_str()) == Some(&type_.get_dir())
@@ -159,7 +161,8 @@ async fn delete(
     let config = config
         .read()
         .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?;
-    let path = config.server.upload_path.join(&*file);
+    let path = safe_path_join(&config.server.upload_path, &*file)
+        .ok_or(error::ErrorInternalServerError("Invalid filename"))?;
     let path = util::glob_match_file(path)?;
     if !path.is_file() || !path.exists() {
         return Err(error::ErrorNotFound("file is not found or expired :(\n"));

src/util.rs

@@ -108,6 +108,21 @@ pub fn sha256_digest<R: Read>(input: R) -> Result<String, ActixError> {
         })?)
 }
 
+/// Joins the paths whilst ensuring the path doesn't drastically change
+pub fn safe_path_join<B: AsRef<Path>, P: AsRef<Path>>(base: B, part: P) -> Option<PathBuf> {
+    if part.as_ref().to_string_lossy().contains("..") {
+        return None;
+    }
+
+    let new_path = base.as_ref().join(part);
+
+    if !new_path.starts_with(base) {
+        return None;
+    }
+
+    Some(new_path)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -170,4 +185,18 @@ mod tests {
         assert_eq!(Vec::<PathBuf>::new(), get_expired_files(&current_dir));
         Ok(())
     }
+
+    #[test]
+    fn test_safe_join_path() {
+        assert!(safe_path_join("/foo", "bar").is_some());
+        assert_eq!(
+            safe_path_join("/foo/bar", "baz/"),
+            Some("/foo/bar/baz/".into())
+        );
+
+        assert!(safe_path_join("/foo", "/foobar").is_none());
+        assert!(safe_path_join("/foo", "/bar").is_none());
+        assert!(safe_path_join("/foo/bar", "..").is_none());
+        assert!(safe_path_join("/foo/bar", "../").is_none());
+    }
 }