fix: do not create directories outside

ports alexcrichton/tar-rs#259
dignifiedquire · Aug 23, 2021 · f1b5899 · f1b5899
1 parent eebfc03
commit f1b5899
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 8 deletions.
diff --git a/src/entry.rs b/src/entry.rs
@@ -470,11 +470,9 @@ impl<R: Read + Unpin> EntryFields<R> {
             None => return Ok(false),
         };
 
-        if parent.symlink_metadata().await.is_err() {
-            fs::create_dir_all(&parent).await.map_err(|e| {
-                TarError::new(&format!("failed to create `{}`", parent.display()), e)
-            })?;
-        }
+        self.ensure_dir_created(&dst, parent).await.map_err(|e| {
+            TarError::new(&format!("failed to create `{}`", parent.display()), dbg!(e))
+        })?;
 
         let canon_target = self.validate_inside_dst(dst, parent).await?;
 
@@ -819,18 +817,38 @@ impl<R: Read + Unpin> EntryFields<R> {
         }
     }
 
+    async fn ensure_dir_created(&self, dst: &Path, dir: &Path) -> io::Result<()> {
+        let mut ancestor = dir;
+        let mut dirs_to_create = Vec::new();
+        while ancestor.symlink_metadata().await.is_err() {
+            dirs_to_create.push(ancestor);
+            if let Some(parent) = ancestor.parent() {
+                ancestor = parent;
+            } else {
+                break;
+            }
+        }
+        for ancestor in dirs_to_create.into_iter().rev() {
+            if let Some(parent) = ancestor.parent() {
+                self.validate_inside_dst(dst, parent).await?;
+            }
+            fs::create_dir(ancestor).await?;
+        }
+        Ok(())
+    }
+
     async fn validate_inside_dst(&self, dst: &Path, file_dst: &Path) -> io::Result<PathBuf> {
         // Abort if target (canonical) parent is outside of `dst`
         let canon_parent = file_dst.canonicalize().await.map_err(|err| {
             Error::new(
                 err.kind(),
-                format!("{} while canonicalizing {}", err, file_dst.display()),
+                format!("{} while canonicalizing {}", dbg!(err), file_dst.display()),
             )
         })?;
         let canon_target = dst.canonicalize().await.map_err(|err| {
             Error::new(
                 err.kind(),
-                format!("{} while canonicalizing {}", err, dst.display()),
+                format!("{} while canonicalizing {}", dbg!(err), dst.display()),
             )
         })?;
         if !canon_parent.starts_with(&canon_target) {

diff --git a/tests/entry.rs b/tests/entry.rs
@@ -1,7 +1,10 @@
 extern crate async_tar;
 extern crate tempfile;
 
-use async_std::{fs::File, prelude::*};
+use async_std::{
+    fs::{create_dir, File},
+    prelude::*,
+};
 
 use tempfile::Builder;
 
@@ -218,6 +221,36 @@ async fn modify_link_just_created() {
     t!(File::open(td.path().join("foo/bar")).await);
 }
 
+#[async_std::test]
+#[cfg(not(windows))] // dangling symlinks have weird permissions
+async fn modify_outside_with_relative_symlink() {
+    let mut ar = async_tar::Builder::new(Vec::new());
+
+    let mut header = async_tar::Header::new_gnu();
+    header.set_size(0);
+    header.set_entry_type(async_tar::EntryType::Symlink);
+    t!(header.set_path("symlink"));
+    t!(header.set_link_name(".."));
+    header.set_cksum();
+    t!(ar.append(&header, &[][..]).await);
+
+    let mut header = async_tar::Header::new_gnu();
+    header.set_size(0);
+    header.set_entry_type(async_tar::EntryType::Regular);
+    t!(header.set_path("symlink/foo/bar"));
+    header.set_cksum();
+    t!(ar.append(&header, &[][..]).await);
+
+    let bytes = t!(ar.into_inner().await);
+    let ar = async_tar::Archive::new(&bytes[..]);
+
+    let td = t!(Builder::new().prefix("tar").tempdir());
+    let tar_dir = td.path().join("tar");
+    create_dir(&tar_dir).await.unwrap();
+    assert!(ar.unpack(tar_dir).await.is_err());
+    assert!(!td.path().join("foo").exists());
+}
+
 #[async_std::test]
 async fn parent_paths_error() {
     let mut ar = async_tar::Builder::new(Vec::new());

diff --git a/tests/header/mod.rs b/tests/header/mod.rs
@@ -153,6 +153,10 @@ fn set_path() {
     assert!(h.set_path(&medium2).is_err());
     assert!(h.set_path("\0").is_err());
 
+    assert!(h.set_path("..").is_err());
+    assert!(h.set_path("foo/..").is_err());
+    assert!(h.set_path("foo/../bar").is_err());
+
     h = Header::new_ustar();
     t!(h.set_path("foo"));
     assert_eq!(t!(h.path()).to_str(), Some("foo"));