From 57a4495d924ca29591fcdf5d9084b5f62e13c23a Mon Sep 17 00:00:00 2001
From: Damien Elmes <gpg@ankiweb.net>
Date: Thu, 17 Mar 2022 16:01:23 +1000
Subject: [PATCH] Check for attempted path traversal on import

---
 .../import_export/package/colpkg/import.rs    | 46 +++++++++++++++++--
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/rslib/src/import_export/package/colpkg/import.rs b/rslib/src/import_export/package/colpkg/import.rs
index 03229bca8..99506f929 100644
--- a/rslib/src/import_export/package/colpkg/import.rs
+++ b/rslib/src/import_export/package/colpkg/import.rs
@@ -5,7 +5,7 @@ use std::{
     collections::HashMap,
     fs::{self, File},
     io::{self, Read, Write},
-    path::{Path, PathBuf},
+    path::{Component, Path, PathBuf},
 };
 
 use prost::Message;
@@ -74,7 +74,8 @@ pub fn import_colpkg(
     check_collection(tempfile.path())?;
     progress_fn(ImportProgress::Collection)?;
 
-    let media_import_result = restore_media(&meta, progress_fn, &mut archive, target_media_folder);
+    let media_folder = Path::new(target_media_folder);
+    let media_import_result = restore_media(&meta, progress_fn, &mut archive, media_folder);
 
     // Proceed with replacing collection, regardless of media import result
     tempfile.as_file().sync_all()?;
@@ -104,7 +105,7 @@ fn restore_media(
     meta: &Meta,
     mut progress_fn: impl FnMut(ImportProgress) -> Result<()>,
     archive: &mut ZipArchive<File>,
-    media_folder: &str,
+    media_folder: &Path,
 ) -> Result<()> {
     let media_entries = extract_media_entries(meta, archive)?;
     std::fs::create_dir_all(media_folder)?;
@@ -117,7 +118,9 @@ fn restore_media(
         }
 
         if let Ok(mut zip_file) = archive.by_name(&archive_file_name.to_string()) {
-            let file_path = Path::new(&media_folder).join(normalize_to_nfc(&entry.name).as_ref());
+            check_filename_safe(&entry.name)?;
+            let nfc_name = normalize_to_nfc(&entry.name);
+            let file_path = media_folder.join(nfc_name.as_ref());
             let size_in_colpkg = if meta.media_list_is_hashmap() {
                 zip_file.size()
             } else {
@@ -148,6 +151,20 @@ fn restore_media(
     Ok(())
 }
 
+/// Return an error if name contains any path separators.
+fn check_filename_safe(name: &str) -> Result<()> {
+    let mut components = Path::new(name).components();
+    let first_element_normal = components
+        .next()
+        .map(|component| matches!(component, Component::Normal(_)))
+        .unwrap_or_default();
+    if !first_element_normal || components.next().is_some() {
+        Err(AnkiError::ImportError(ImportError::Corrupt))
+    } else {
+        Ok(())
+    }
+}
+
 fn extract_media_entries(meta: &Meta, archive: &mut ZipArchive<File>) -> Result<Vec<MediaEntry>> {
     let mut file = archive.by_name("media")?;
     let mut buf = Vec::new();
@@ -201,3 +218,24 @@ fn copy_collection(
 
     Ok(())
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn path_traversal() {
+        assert!(check_filename_safe("foo").is_ok(),);
+
+        assert!(check_filename_safe("..").is_err());
+        assert!(check_filename_safe("foo/bar").is_err());
+        assert!(check_filename_safe("/foo").is_err());
+        assert!(check_filename_safe("../foo").is_err());
+
+        if cfg!(windows) {
+            assert!(check_filename_safe("foo\\bar").is_err());
+            assert!(check_filename_safe("c:\\foo").is_err());
+            assert!(check_filename_safe("\\foo").is_err());
+        }
+    }
+}