Validate tarball components

5 files changed, 27 insertions, 5 deletions

diff --git a/nix-rust/src/c.rs b/nix-rust/src/c.rs
index 19e737a88..3feeb71a3 100644
--- a/nix-rust/src/c.rs
+++ b/nix-rust/src/c.rs
@@ -11,7 +11,10 @@ pub extern "C" fn unpack_tarfile(
     source: foreign::Source,
     dest_dir: &str,
 ) -> CBox<Result<(), error::CppException>> {
-    CBox::new(util::tarfile::unpack_tarfile(source, dest_dir).map_err(|err| err.into()))
+    CBox::new(
+        util::tarfile::unpack_tarfile(source, std::path::Path::new(dest_dir))
+            .map_err(|err| err.into()),
+    )
 }
 
 #[no_mangle]
diff --git a/nix-rust/src/error.rs b/nix-rust/src/error.rs
index c2ba5d504..586a8f53d 100644
--- a/nix-rust/src/error.rs
+++ b/nix-rust/src/error.rs
@@ -23,6 +23,7 @@ pub enum Error {
     HttpError(hyper::error::Error),
     Misc(String),
     Foreign(CppException),
+    BadTarFileMemberName(String),
 }
 
 impl From<std::io::Error> for Error {
@@ -64,6 +65,9 @@ impl fmt::Display for Error {
             Error::HttpError(err) => write!(f, "HTTP error: {}", err),
             Error::Foreign(_) => write!(f, "<C++ exception>"), // FIXME
             Error::Misc(s) => write!(f, "{}", s),
+            Error::BadTarFileMemberName(s) => {
+                write!(f, "tar archive contains illegal file name '{}'", s)
+            }
         }
     }
 }
diff --git a/nix-rust/src/util/tarfile.rs b/nix-rust/src/util/tarfile.rs
index 379d9098f..74d60692c 100644
--- a/nix-rust/src/util/tarfile.rs
+++ b/nix-rust/src/util/tarfile.rs
@@ -2,18 +2,28 @@ use crate::{foreign::Source, Error};
 use std::fs;
 use std::io;
 use std::os::unix::fs::OpenOptionsExt;
-use std::path::Path;
+use std::path::{Component, Path};
 use tar::Archive;
 
-pub fn unpack_tarfile(source: Source, dest_dir: &str) -> Result<(), Error> {
-    let dest_dir = Path::new(dest_dir);
+pub fn unpack_tarfile(source: Source, dest_dir: &Path) -> Result<(), Error> {
+    fs::create_dir_all(dest_dir)?;
 
     let mut tar = Archive::new(source);
 
     for file in tar.entries()? {
         let mut file = file?;
 
-        let dest_file = dest_dir.join(file.path()?);
+        let path = file.path()?;
+
+        for i in path.components() {
+            if let Component::Prefix(_) | Component::RootDir | Component::ParentDir = i {
+                return Err(Error::BadTarFileMemberName(
+                    file.path()?.to_str().unwrap().to_string(),
+                ));
+            }
+        }
+
+        let dest_file = dest_dir.join(path);
 
         fs::create_dir_all(dest_file.parent().unwrap())?;
 
diff --git a/tests/bad.tar.xz b/tests/bad.tar.xz
new file mode 100644
index 000000000..250a5ad1a
--- /dev/null
+++ b/tests/bad.tar.xz
diff --git a/tests/tarball.sh b/tests/tarball.sh
index 7738b948d..8adb8d72f 100644
--- a/tests/tarball.sh
+++ b/tests/tarball.sh
@@ -35,3 +35,8 @@ test_tarball() {
 test_tarball '' cat
 test_tarball .xz xz
 test_tarball .gz gzip
+
+rm -rf $TEST_ROOT/tmp
+mkdir -p $TEST_ROOT/tmp
+(! TMPDIR=$TEST_ROOT/tmp XDG_RUNTIME_DIR=$TEST_ROOT/tmp nix-env -f file://$(pwd)/bad.tar.xz -qa --out-path)
+(! [ -e $TEST_ROOT/tmp/bad ])