change untar to use unpack instead of unpack_in (#19216)

* change untar to use unpack instead of unpack_in * hacky, but maybe passes tests * chore: bump tar from 0.4.35 to 0.4.37 Bumps [tar](https://github.com/alexcrichton/tar-rs) from 0.4.35 to 0.4.37. - [Release notes](https://github.com/alexcrichton/tar-rs/releases) - [Commits](alexcrichton/tar-rs@0.4.35...0.4.37) --- updated-dependencies: - dependency-name: tar dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> * [auto-commit] Update all Cargo lock files * cleanup * cleanup, add validate_inside_dst * collapse use Co-authored-by: Tyera Eulberg <[email protected]> * delete comment line * add comments Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite <[email protected]> Co-authored-by: Tyera Eulberg <[email protected]> (cherry picked from commit 89a31ff) # Conflicts: # Cargo.lock # download-utils/Cargo.toml # install/Cargo.toml # programs/bpf/Cargo.lock # runtime/Cargo.toml # sdk/cargo-build-bpf/Cargo.toml
solana-labs · Aug 20, 2021 · e906fd3 · e906fd3
1 parent b5b1ed2
commit e906fd3
Show file tree

Hide file tree

Showing 7 changed files with 138 additions and 4 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/download-utils/Cargo.toml b/download-utils/Cargo.toml
@@ -12,12 +12,21 @@ edition = "2018"
 [dependencies]
 bzip2 = "0.3.3"
 console = "0.14.1"
+<<<<<<< HEAD
 indicatif = "0.15.0"
 log = "0.4.11"
 reqwest = { version = "0.11.2", default-features = false, features = ["blocking", "rustls-tls", "json"] }
 solana-sdk = { path = "../sdk", version = "=1.7.11" }
 solana-runtime = { path = "../runtime", version = "=1.7.11" }
 tar = "0.4.28"
+=======
+indicatif = "0.16.2"
+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.37"
+>>>>>>> 89a31ff47 (change untar to use unpack instead of unpack_in (#19216))
 
 [lib]
 crate-type = ["lib"]

diff --git a/install/Cargo.toml b/install/Cargo.toml
@@ -20,6 +20,7 @@ ctrlc = { version = "3.1.5", features = ["termination"] }
 dirs-next = "2.0.0"
 indicatif = "0.15.0"
 lazy_static = "1.4.0"
+<<<<<<< HEAD
 nix = "0.19.0"
 reqwest = { version = "0.11.2", default-features = false, features = ["blocking", "rustls-tls", "json"] }
 serde = { version = "1.0.122", features = ["derive"] }
@@ -35,6 +36,23 @@ semver = "0.9.0"
 tar = "0.4.28"
 tempfile = "3.1.0"
 url = "2.1.1"
+=======
+nix = "0.20.0"
+reqwest = { version = "0.11.4", default-features = false, features = ["blocking", "rustls-tls", "json"] }
+serde = { version = "1.0.127", features = ["derive"] }
+serde_json = "1.0.66"
+serde_yaml = "0.8.18"
+solana-clap-utils = { path = "../clap-utils", version = "=1.8.0" }
+solana-client = { path = "../client", version = "=1.8.0" }
+solana-config-program = { path = "../programs/config", version = "=1.8.0" }
+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.37"
+tempfile = "3.2.0"
+url = "2.2.2"
+>>>>>>> 89a31ff47 (change untar to use unpack instead of unpack_in (#19216))
 
 [target."cfg(windows)".dependencies]
 winapi = "0.3.8"

diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock
diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml
@@ -48,8 +48,13 @@ solana-secp256k1-program = { path = "../programs/secp256k1", version = "=1.7.11"
 solana-stake-program = { path = "../programs/stake", version = "=1.7.11" }
 solana-vote-program = { path = "../programs/vote", version = "=1.7.11" }
 symlink = "0.1.0"
+<<<<<<< HEAD
 tar = "0.4.28"
 tempfile = "3.1.0"
+=======
+tar = "0.4.37"
+tempfile = "3.2.0"
+>>>>>>> 89a31ff47 (change untar to use unpack instead of unpack_in (#19216))
 thiserror = "1.0"
 zstd = "0.5.1"
 

diff --git a/runtime/src/hardened_unpack.rs b/runtime/src/hardened_unpack.rs
@@ -9,7 +9,7 @@ use {
         fs::{self, File},
         io::{BufReader, Read},
         path::{
-            Component::{CurDir, Normal},
+            Component::{self, CurDir, Normal},
             Path, PathBuf,
         },
         time::Instant,
@@ -161,9 +161,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 +204,80 @@ 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
+    // 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)?;
+
+    // Here we are different than untar_in. The code for tar::unpack_in internally calling unpack is a little different.
+    // ignore return value here
+    validate_inside_dst(dst, parent)?;
+    let target = parent.join(entry_path.file_name().unwrap());
+
+    Ok(Some(target))
+}
+
+// copied from:
+// https://github.com/alexcrichton/tar-rs/blob/d90a02f582c03dfa0fd11c78d608d0974625ae5d/src/entry.rs#L781
+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>;
 

diff --git a/sdk/cargo-build-bpf/Cargo.toml b/sdk/cargo-build-bpf/Cargo.toml
@@ -12,11 +12,22 @@ publish = false
 [dependencies]
 bzip2 = "0.3.3"
 clap = "2.33.3"
+<<<<<<< HEAD
 regex = "1.4.5"
 cargo_metadata = "0.12.0"
 solana-sdk = { path = "..", version = "=1.7.11" }
 solana-download-utils = { path = "../../download-utils", version = "=1.7.11" }
 tar = "0.4.28"
+=======
+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.37"
+
+[dev-dependencies]
+serial_test = "*"
+>>>>>>> 89a31ff47 (change untar to use unpack instead of unpack_in (#19216))
 
 [features]
 program = []