From f1b589981dcf6fbdd14ad0477612d4be80d66cb9 Mon Sep 17 00:00:00 2001 From: dignifiedquire Date: Mon, 23 Aug 2021 21:28:30 +0200 Subject: [PATCH] fix: do not create directories outside ports https://github.com/alexcrichton/tar-rs/pull/259 --- src/entry.rs | 32 +++++++++++++++++++++++++------- tests/entry.rs | 35 ++++++++++++++++++++++++++++++++++- tests/header/mod.rs | 4 ++++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/entry.rs b/src/entry.rs index ab24c23..b7c3fdd 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -470,11 +470,9 @@ impl EntryFields { 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 EntryFields { } } + 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 { // 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 index 3ea22aa..788fed2 100644 --- 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 index 31a5439..ad65478 100644 --- 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"));