Added tests and better path validation

Secure video endpoints.

src/files.rs

@@ -30,7 +30,109 @@ fn is_image_or_video(path: &Path) -> bool {
     extension == "png"
         || extension == "jpg"
         || extension == "jpeg"
-        || extension == "rs"
         || extension == "mp4"
         || extension == "mov"
 }
+
+pub fn is_valid_path(path: &str) -> Option<PathBuf> {
+    let base = PathBuf::from(dotenv::var("BASE_PATH").unwrap());
+
+    is_valid_full_path(&base, path)
+}
+
+fn is_valid_full_path(base: &Path, path: &str) -> Option<PathBuf> {
+    let path = PathBuf::from(path);
+    if path.is_relative() {
+        let mut full_path = PathBuf::from(base);
+        full_path.push(&path);
+        full_path
+            .canonicalize()
+            .and_then(|p| {
+                if p.starts_with(base) {
+                    Ok(p)
+                } else {
+                    Err(io::Error::new(
+                        io::ErrorKind::Other,
+                        "Path below base directory",
+                    ))
+                }
+            })
+            .ok()
+    } else {
+        None
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::env;
+    use std::fs::{create_dir_all, File};
+
+    #[test]
+    fn directory_traversal_test() {
+        assert_eq!(None, is_valid_path("../"));
+        assert_eq!(None, is_valid_path(".."));
+        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"));
+    }
+
+    #[test]
+    fn build_from_relative_path_test() {
+        let base = env::temp_dir();
+        let mut test_file = PathBuf::from(&base);
+        test_file.push("test.png");
+        File::create(test_file).unwrap();
+
+        assert!(is_valid_full_path(&base, "test.png").is_some());
+
+        let path = "relative/path/test.png";
+        let mut test_file = PathBuf::from(&base);
+        test_file.push(path);
+        create_dir_all(test_file.parent().unwrap()).unwrap();
+        File::create(test_file).unwrap();
+
+        assert_eq!(
+            Some(PathBuf::from("/tmp/relative/path/test.png")),
+            is_valid_full_path(&base, path)
+        );
+    }
+
+    #[test]
+    fn png_valid_extension_test() {
+        assert!(is_image_or_video(Path::new("image.png")));
+        assert!(is_image_or_video(Path::new("image.PNG")));
+        assert!(is_image_or_video(Path::new("image.pNg")));
+    }
+
+    #[test]
+    fn jpg_valid_extension_test() {
+        assert!(is_image_or_video(Path::new("image.jpeg")));
+        assert!(is_image_or_video(Path::new("image.JPEG")));
+        assert!(is_image_or_video(Path::new("image.jpg")));
+        assert!(is_image_or_video(Path::new("image.JPG")));
+    }
+
+    #[test]
+    fn mp4_valid_extension_test() {
+        assert!(is_image_or_video(Path::new("image.mp4")));
+        assert!(is_image_or_video(Path::new("image.mP4")));
+        assert!(is_image_or_video(Path::new("image.MP4")));
+    }
+
+    #[test]
+    fn mov_valid_extension_test() {
+        assert!(is_image_or_video(Path::new("image.mov")));
+        assert!(is_image_or_video(Path::new("image.MOV")));
+        assert!(is_image_or_video(Path::new("image.MoV")));
+    }
+
+    #[test]
+    fn hidden_file_not_valid_test() {
+        assert!(!is_image_or_video(Path::new(".DS_store")));
+    }
+}

src/main.rs

@@ -14,7 +14,7 @@ use std::path::{Path, PathBuf};
 
 use crate::data::{Claims, CreateAccountRequest, Token};
 use crate::database::{create_user, get_user, user_exists};
-use crate::files::list_files;
+use crate::files::{is_valid_path, list_files};
 use crate::video::*;
 
 mod data;
@@ -89,23 +89,6 @@ struct PhotosResponse<'a> {
     dirs: &'a [String],
 }
 
-fn is_valid_path(path: &str) -> Option<PathBuf> {
-    match path {
-        path if path.contains("..") => None,
-
-        path => {
-            let path = PathBuf::from(path);
-            if path.is_relative() {
-                let mut full_path = PathBuf::from(dotenv::var("BASE_PATH").unwrap());
-                full_path.push(path);
-                Some(full_path)
-            } else {
-                None
-            }
-        }
-    }
-}
-
 #[get("/image")]
 async fn get_image(
     _claims: Claims,
@@ -156,7 +139,7 @@ async fn generate_video(_claims: Claims, body: web::Json<ThumbnailRequest>) -> i
 }
 
 #[get("/video/stream")]
-async fn stream_video(request: HttpRequest, path: web::Query<ThumbnailRequest>) -> impl Responder {
+async fn stream_video(request: HttpRequest, _: Claims, path: web::Query<ThumbnailRequest>) -> impl Responder {
     let playlist = &path.path;
     println!("Playlist: {}", playlist);
 
@@ -168,7 +151,7 @@ async fn stream_video(request: HttpRequest, path: web::Query<ThumbnailRequest>)
 }
 
 #[get("/video/{path}")]
-async fn get_video_part(request: HttpRequest, path: web::Path<ThumbnailRequest>) -> impl Responder {
+async fn get_video_part(request: HttpRequest, _: Claims , path: web::Path<ThumbnailRequest>) -> impl Responder {
     let part = &path.path;
     println!("Video part: {}", part);
 
@@ -209,7 +192,8 @@ async fn create_thumbnails() {
                 if ext == "mp4" || ext == "mov" {
                     let relative_path = &entry.path().strip_prefix(&images).unwrap();
                     let thumb_path = Path::new(thumbnail_directory).join(relative_path);
-                    std::fs::create_dir_all(&thumb_path.parent().unwrap()).expect("Error creating directory");
+                    std::fs::create_dir_all(&thumb_path.parent().unwrap())
+                        .expect("Error creating directory");
                     generate_video_thumbnail(entry.path(), &thumb_path);
                     false
                 } else {