From ef336a44a155c0999f1d41ebfbf627d89de1f979 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 20 Aug 2021 17:13:35 +0000 Subject: [PATCH] change untar to use unpack instead of unpack_in (backport #19216) (#19340) * 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](https://github.com/alexcrichton/tar-rs/compare/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] * [auto-commit] Update all Cargo lock files * cleanup * cleanup, add validate_inside_dst * collapse use Co-authored-by: Tyera Eulberg * delete comment line * add comments Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot-buildkite Co-authored-by: Tyera Eulberg (cherry picked from commit 89a31ff473470513e3855edaa1c73cdedef0643d) # Conflicts: # Cargo.lock # download-utils/Cargo.toml # install/Cargo.toml # programs/bpf/Cargo.lock # runtime/Cargo.toml # sdk/cargo-build-bpf/Cargo.toml * Fix conflicts Co-authored-by: Jeff Washington (jwash) <75863576+jeffwashington@users.noreply.github.com> Co-authored-by: Tyera Eulberg --- Cargo.lock | 5 +- download-utils/Cargo.toml | 2 +- install/Cargo.toml | 2 +- programs/bpf/Cargo.lock | 5 +- runtime/Cargo.toml | 2 +- runtime/src/hardened_unpack.rs | 87 ++++++++++++++++++++++++++++++++-- sdk/cargo-build-bpf/Cargo.toml | 2 +- 7 files changed, 91 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7db7c6ec4b..d739ddd7c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6000,13 +6000,12 @@ dependencies = [ [[package]] name = "tar" -version = "0.4.28" +version = "0.4.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c058ad0bd6ccb84faa24cc44d4fc99bee8a5d7ba9ff33aa4d993122d1aeeac2" +checksum = "d6f5515d3add52e0bbdcad7b83c388bb36ba7b754dda3b5f5bc2d38640cdba5c" dependencies = [ "filetime", "libc", - "redox_syscall", "xattr", ] diff --git a/download-utils/Cargo.toml b/download-utils/Cargo.toml index a4ec738500..808f808fd5 100644 --- a/download-utils/Cargo.toml +++ b/download-utils/Cargo.toml @@ -17,7 +17,7 @@ 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" +tar = "0.4.37" [lib] crate-type = ["lib"] diff --git a/install/Cargo.toml b/install/Cargo.toml index ff1b849744..9976677eab 100644 --- a/install/Cargo.toml +++ b/install/Cargo.toml @@ -32,7 +32,7 @@ solana-logger = { path = "../logger", version = "=1.7.11" } solana-sdk = { path = "../sdk", version = "=1.7.11" } solana-version = { path = "../version", version = "=1.7.11" } semver = "0.9.0" -tar = "0.4.28" +tar = "0.4.37" tempfile = "3.1.0" url = "2.1.1" diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 3d2b96c132..5ac4d1e5d1 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -3820,13 +3820,12 @@ dependencies = [ [[package]] name = "tar" -version = "0.4.29" +version = "0.4.37" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8a4c1d0bee3230179544336c15eefb563cf0302955d962e456542323e8c2e8a" +checksum = "d6f5515d3add52e0bbdcad7b83c388bb36ba7b754dda3b5f5bc2d38640cdba5c" dependencies = [ "filetime", "libc", - "redox_syscall 0.1.56", "xattr", ] diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 3c306ed9a3..355bcafa55 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -48,7 +48,7 @@ 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" -tar = "0.4.28" +tar = "0.4.37" tempfile = "3.1.0" thiserror = "1.0" zstd = "0.5.1" diff --git a/runtime/src/hardened_unpack.rs b/runtime/src/hardened_unpack.rs index fdaa424f6c..7580ab4b10 100644 --- 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> { + // 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> = 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 { + // 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; diff --git a/sdk/cargo-build-bpf/Cargo.toml b/sdk/cargo-build-bpf/Cargo.toml index f619b2e55c..a2156eff0c 100644 --- a/sdk/cargo-build-bpf/Cargo.toml +++ b/sdk/cargo-build-bpf/Cargo.toml @@ -16,7 +16,7 @@ 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" +tar = "0.4.37" [features] program = []