Avoid creating directories outside of target (#2)

## Summary

This is a port of alexcrichton/tar-rs#259 which was later ported to `async-tar` in dignifiedquire/async-tar#24. The goal is to avoid allowing archives to create directories outside of the target path by deferring the creation of directories.

## Test Plan

`cargo test`

src/archive.rs

@@ -11,6 +11,7 @@ use portable_atomic::{
     Ordering,
 };
 use tokio::{
+    fs,
     io::{self, AsyncRead as Read, AsyncReadExt},
     sync::Mutex,
 };
@@ -226,10 +227,40 @@ impl<R: Read + Unpin> Archive<R> {
     pub async fn unpack<P: AsRef<Path>>(&mut self, dst: P) -> io::Result<()> {
         let mut entries = self.entries()?;
         let mut pinned = Pin::new(&mut entries);
+        let dst = dst.as_ref();
+
+        if fs::symlink_metadata(dst).await.is_err() {
+            fs::create_dir_all(&dst)
+                .await
+                .map_err(|e| TarError::new(&format!("failed to create `{}`", dst.display()), e))?;
+        }
+
+        // Canonicalizing the dst directory will prepend the path with '\\?\'
+        // on windows which will allow windows APIs to treat the path as an
+        // extended-length path with a 32,767 character limit. Otherwise all
+        // unpacked paths over 260 characters will fail on creation with a
+        // NotFound exception.
+        let dst = fs::canonicalize(dst)
+            .await
+            .unwrap_or_else(|_| dst.to_path_buf());
+
+        // Delay any directory entries until the end (they will be created if needed by
+        // descendants), to ensure that directory permissions do not interfer with descendant
+        // extraction.
+        let mut directories = Vec::new();
         while let Some(entry) = pinned.next().await {
             let mut file = entry.map_err(|e| TarError::new("failed to iterate over archive", e))?;
-            file.unpack_in(dst.as_ref()).await?;
+            if file.header().entry_type() == crate::EntryType::Directory {
+                directories.push(file);
+            } else {
+                file.unpack_in(&dst).await?;
+            }
         }
+
+        for mut dir in directories {
+            dir.unpack_in(&dst).await?;
+        }
+
         Ok(())
     }
 }

src/entry.rs

@@ -477,11 +477,9 @@ impl<R: Read + Unpin> EntryFields<R> {
             None => return Ok(None),
         };
 
-        if parent.symlink_metadata().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()), e))?;
 
         let canon_target = self.validate_inside_dst(dst, parent).await?;
 
@@ -864,6 +862,26 @@ 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 tokio::fs::symlink_metadata(ancestor).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().map_err(|err| {

tests/entry.rs

@@ -220,6 +220,35 @@ async fn modify_link_just_created() {
     t!(File::open(td.path().join("foo/bar")).await);
 }
 
+#[tokio::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 mut ar = async_tar::Archive::new(&bytes[..]);
+
+    let td = t!(Builder::new().prefix("tar").tempdir());
+    let tar_dir = td.path().join("tar");
+    assert!(ar.unpack(tar_dir).await.is_err());
+    assert!(!td.path().join("foo").exists());
+}
+
 #[tokio::test]
 async fn parent_paths_error() {
     let mut ar = async_tar::Builder::new(Vec::new());

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"));