Don't create directories before we've verified they're inside the tar…

… destination (#259)

Fixes #238

Co-authored-by: Robert O'Callahan <robert@ocallahan.org>

src/entry.rs

@@ -413,11 +413,8 @@ impl<'a> EntryFields<'a> {
             None => return Ok(false),
         };
 
-        if parent.symlink_metadata().is_err() {
-            fs::create_dir_all(&parent).map_err(|e| {
-                TarError::new(&format!("failed to create `{}`", parent.display()), e)
-            })?;
-        }
+        self.ensure_dir_created(&dst, parent)
+            .map_err(|e| TarError::new(&format!("failed to create `{}`", parent.display()), e))?;
 
         let canon_target = self.validate_inside_dst(&dst, parent)?;
 
@@ -761,6 +758,26 @@ impl<'a> EntryFields<'a> {
         }
     }
 
+    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().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)?;
+            }
+            fs::create_dir(ancestor)?;
+        }
+        Ok(())
+    }
+
     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

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

tests/header/mod.rs

@@ -154,6 +154,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"));