Skip to content

Commit

Permalink
fix: do not create directories outside (#24)
Browse files Browse the repository at this point in the history
* fix: do not create directories outside

ports alexcrichton/tar-rs#259

* fixups
  • Loading branch information
dignifiedquire committed Aug 24, 2021
1 parent eebfc03 commit 9e61431
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 8 deletions.
34 changes: 32 additions & 2 deletions src/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{
};

use async_std::{
io,
fs, io,
io::prelude::*,
path::Path,
prelude::*,
Expand Down Expand Up @@ -224,10 +224,40 @@ impl<R: Read + Unpin> Archive<R> {
pub async fn unpack<P: AsRef<Path>>(self, dst: P) -> io::Result<()> {
let mut entries = self.entries()?;
let mut pinned = Pin::new(&mut entries);
let dst = dst.as_ref();

if dst.symlink_metadata().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 = &dst
.canonicalize()
.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(())
}
}
Expand Down
28 changes: 23 additions & 5 deletions src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()), e))?;

let canon_target = self.validate_inside_dst(dst, parent).await?;

Expand Down Expand Up @@ -819,6 +817,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 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| {
Expand Down
35 changes: 34 additions & 1 deletion tests/entry.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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());
Expand Down
4 changes: 4 additions & 0 deletions tests/header/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down

0 comments on commit 9e61431

Please sign in to comment.