From 5a6d6da7aea23a48126a77f98612518af66bc203 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 13 Dec 2019 18:11:37 +0100
Subject: [PATCH] Validate tarball components

---
 nix-rust/src/c.rs            |   5 ++++-
 nix-rust/src/error.rs        |   4 ++++
 nix-rust/src/util/tarfile.rs |  18 ++++++++++++++----
 tests/bad.tar.xz             | Bin 0 -> 228 bytes
 tests/tarball.sh             |   5 +++++
 5 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 tests/bad.tar.xz

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 0000000000000000000000000000000000000000..250a5ad1a79ee088d5976160664daad6e1a136ff
GIT binary patch
literal 228
zcmV<A02}}PH+ooF000E$*0e?f03iVu0001VFXf})C;tGVT>uwgxWr*GQJ}UxbD$dT
zFZcR4Oq`4c+brLa?D6R=fMJjM$?MdlXt0Pp!zwDzlSF=e({Qq}bnc_B9C}zwW!F?<
z;h>7LqPxu=kzFWj+$Z*4&0g78Yyp1kn1+nCzFC}~s1p<w+rfbIGz<bH?U#Qzyw`;6
z`({F^v@!)B&J6Y%vn53KeRqd?V5+Eub*M94BlkPr%izV3Qzu-*Z?*3L0002dm00(D
e^?nKf0loo%Pyhhq7Z6{u#Ao{g000001X)^+32KG_

literal 0
HcmV?d00001

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 ])