change untar to use unpack instead of unpack_in

download-utils/Cargo.toml

@@ -17,7 +17,7 @@ log = "0.4.14"
 reqwest = { version = "0.11.4", default-features = false, features = ["blocking", "rustls-tls", "json"] }
 solana-sdk = { path = "../sdk", version = "=1.8.0" }
 solana-runtime = { path = "../runtime", version = "=1.8.0" }
-tar = "0.4.35"
+tar = "0.4.37"
 
 [lib]
 crate-type = ["lib"]

install/Cargo.toml

@@ -32,7 +32,7 @@ solana-logger = { path = "../logger", version = "=1.8.0" }
 solana-sdk = { path = "../sdk", version = "=1.8.0" }
 solana-version = { path = "../version", version = "=1.8.0" }
 semver = "1.0.4"
-tar = "0.4.35"
+tar = "0.4.37"
 tempfile = "3.2.0"
 url = "2.2.2"
 

runtime/Cargo.toml

@@ -49,7 +49,7 @@ solana-secp256k1-program = { path = "../programs/secp256k1", version = "=1.8.0"
 solana-stake-program = { path = "../programs/stake", version = "=1.8.0" }
 solana-vote-program = { path = "../programs/vote", version = "=1.8.0" }
 symlink = "0.1.0"
-tar = "0.4.35"
+tar = "0.4.37"
 tempfile = "3.2.0"
 thiserror = "1.0"
 zstd = "0.9.0"

runtime/src/hardened_unpack.rs

@@ -9,6 +9,7 @@ use {
         fs::{self, File},
         io::{BufReader, Read},
         path::{
+            Component,
             Component::{CurDir, Normal},
             Path, PathBuf,
         },
@@ -161,9 +162,14 @@ where
         )?;
         total_count = checked_total_count_increment(total_count, limit_count)?;
 
-        // unpack_in does its own sanitization
-        // ref: https://docs.rs/tar/*/tar/struct.Entry.html#method.unpack_in
-        check_unpack_result(entry.unpack_in(unpack_dir)?, path_str)?;
+        let target = sanitize_path(&entry.path()?, unpack_dir)?; // ? handles file system errors
+        if target.is_none() {
+            continue; // skip it
+        }
+        let target = target.unwrap();
+
+        let unpack = entry.unpack(target);
+        check_unpack_result(unpack.map(|_unpack| true)?, path_str)?;
 
         // Sanitize permissions.
         let mode = match entry.header().entry_type() {
@@ -199,6 +205,76 @@ where
     }
 }
 
+// return Err on file system error
+// return Some(path) if path is good
+// return None if we should skip this file
+fn sanitize_path(entry_path: &Path, dst: &Path) -> Result<Option<PathBuf>> {
+    // We cannot call unpack_in because it errors if we try to use 2 account paths.
+    // So, this code is borrowed from unpack_in
+    // code from: unpack_in does its own sanitization
+    // ref: https://docs.rs/tar/*/tar/struct.Entry.html#method.unpack_in
+    let mut file_dst = dst.to_path_buf();
+    const SKIP: Result<Option<PathBuf>> = Ok(None);
+    {
+        let path = entry_path;
+        for part in path.components() {
+            match part {
+                // Leading '/' characters, root paths, and '.'
+                // components are just ignored and treated as "empty
+                // components"
+                Component::Prefix(..) | Component::RootDir | Component::CurDir => continue,
+
+                // If any part of the filename is '..', then skip over
+                // unpacking the file to prevent directory traversal
+                // security issues.  See, e.g.: CVE-2001-1267,
+                // CVE-2002-0399, CVE-2005-1918, CVE-2007-4131
+                Component::ParentDir => return SKIP,
+
+                Component::Normal(part) => file_dst.push(part),
+            }
+        }
+    }
+
+    // Skip cases where only slashes or '.' parts were seen, because
+    // this is effectively an empty filename.
+    if *dst == *file_dst {
+        return SKIP;
+    }
+
+    // Skip entries without a parent (i.e. outside of FS root)
+    let parent = match file_dst.parent() {
+        Some(p) => p,
+        None => return SKIP,
+    };
+
+    fs::create_dir_all(parent)?;
+    validate_inside_dst(dst, parent)?;
+    let target = parent.join(entry_path.file_name().unwrap());
+
+    Ok(Some(target))
+}
+
+fn validate_inside_dst(dst: &Path, file_dst: &Path) -> Result<PathBuf> {
+    // Abort if target (canonical) parent is outside of `dst`
+    let canon_parent = file_dst.canonicalize().map_err(|err| {
+        UnpackError::Archive(format!(
+            "{} while canonicalizing {}",
+            err,
+            file_dst.display()
+        ))
+    })?;
+    let canon_target = dst.canonicalize().map_err(|err| {
+        UnpackError::Archive(format!("{} while canonicalizing {}", err, dst.display()))
+    })?;
+    if !canon_parent.starts_with(&canon_target) {
+        return Err(UnpackError::Archive(format!(
+            "trying to unpack outside of destination path: {}",
+            canon_target.display()
+        )));
+    }
+    Ok(canon_target)
+}
+
 /// Map from AppendVec file name to unpacked file system location
 pub type UnpackedAppendVecMap = HashMap<String, PathBuf>;
 

sdk/cargo-build-bpf/Cargo.toml

@@ -16,7 +16,7 @@ regex = "1.5.4"
 cargo_metadata = "0.14.0"
 solana-sdk = { path = "..", version = "=1.8.0" }
 solana-download-utils = { path = "../../download-utils", version = "=1.8.0" }
-tar = "0.4.35"
+tar = "0.4.37"
 
 [dev-dependencies]
 serial_test = "*"