diff --git a/Cargo.lock b/Cargo.lock index 613cce7..e57828a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -439,6 +439,7 @@ dependencies = [ "serde", "serde_derive", "tar", + "tempfile", "time", "toml", "url", @@ -1796,9 +1797,9 @@ dependencies = [ [[package]] name = "tar" -version = "0.4.43" +version = "0.4.46" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c65998313f8e17d0d553d28f91a0df93e4dbbbf770279c7bc21ca0f09ea1a1f6" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" dependencies = [ "filetime", "libc", diff --git a/src/elan-dist/Cargo.toml b/src/elan-dist/Cargo.toml index 5077163..b017813 100644 --- a/src/elan-dist/Cargo.toml +++ b/src/elan-dist/Cargo.toml @@ -28,6 +28,9 @@ serde = "1.0.119" serde_derive = "1.0.119" fslock = "0.2.1" +[dev-dependencies] +tempfile = "3.2.0" + [target."cfg(windows)".dependencies] winapi = { version = "0.3.9", features = ["handleapi", "sysinfoapi", "tlhelp32", "winnt"] } winreg = "0.8.0" diff --git a/src/elan-dist/src/component/package.rs b/src/elan-dist/src/component/package.rs index 89ff5ce..f764e1e 100644 --- a/src/elan-dist/src/component/package.rs +++ b/src/elan-dist/src/component/package.rs @@ -6,7 +6,7 @@ use crate::errors::*; use std::fs::{self, File}; use std::io::{self, Read, Seek}; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; use time::OffsetDateTime; use zip::ZipArchive; @@ -24,6 +24,37 @@ impl TarPackage { } } +/// Whether `component` is a plain file name that cannot take on another meaning +/// when combined with other components, also on Windows (e.g. `C:`, `.. `). +fn is_plain_name(component: Component<'_>) -> bool { + match component { + Component::Normal(name) => { + let mut parsed = Path::new(name).components(); + matches!(parsed.next(), Some(Component::Normal(_))) + && parsed.next().is_none() + && !name.to_string_lossy().ends_with(['.', ' ']) + } + _ => false, + } +} + +/// Returns `path` relative to the unpacking destination, i.e. without its first +/// component, or an error if it could refer to a location outside the destination. +fn strip_first_dir(path: &Path) -> Result { + let mut components = path.components(); + // Throw away the first path component + components.next(); + let mut relpath = PathBuf::new(); + for component in components { + match component { + Component::CurDir => {} + c if is_plain_name(c) => relpath.push(c), + _ => return Err(format!("invalid path in archive: '{}'", path.display()).into()), + } + } + Ok(relpath) +} + fn unpack_without_first_dir(archive: &mut tar::Archive, path: &Path) -> Result<()> { let entries = archive .entries() @@ -35,10 +66,37 @@ fn unpack_without_first_dir(archive: &mut tar::Archive, path: &Path) let path = path.chain_err(|| ErrorKind::ExtractingPackage)?; path.into_owned() }; - let mut components = relpath.components(); - // Throw away the first path component - components.next(); - let full_path = path.join(&components.as_path()); + let stripped = strip_first_dir(&relpath)?; + let kind = entry.header().entry_type(); + // An entry like `pkg` or `pkg/.` refers to the destination itself. Only a + // directory may be unpacked there: anything else would be created at + // `path`, and a symlink there would be resolved relative to the parent + // of `path`, letting later entries escape through it. + if stripped.as_os_str().is_empty() && !kind.is_dir() { + return Err(format!("invalid path in archive: '{}'", relpath.display()).into()); + } + let full_path = path.join(stripped); + + if kind.is_hard_link() { + return Err( + format!("unsupported hard link in archive: '{}'", relpath.display()).into(), + ); + } + if kind.is_symlink() { + // Only allow links to the link's own directory or below it, so that no + // chain of links can lead outside the destination. + let target = entry + .link_name() + .chain_err(|| ErrorKind::ExtractingPackage)?; + let is_contained = target.as_ref().is_some_and(|target| { + target + .components() + .all(|c| c == Component::CurDir || is_plain_name(c)) + }); + if !is_contained { + return Err(format!("invalid symlink in archive: '{}'", relpath.display()).into()); + } + } // Create the full path to the entry if it does not exist already match full_path.parent() { @@ -88,11 +146,12 @@ impl ZipPackage { if entry.name().ends_with('/') { continue; // skip directories } - let relpath = PathBuf::from(entry.name()); - let mut components = relpath.components(); - // Throw away the first path component - components.next(); - let full_path = path.join(&components.as_path()); + let stripped = strip_first_dir(Path::new(entry.name()))?; + // A file entry like `pkg` would be written at the destination itself. + if stripped.as_os_str().is_empty() { + return Err(format!("invalid path in archive: '{}'", entry.name()).into()); + } + let full_path = path.join(stripped); // Create the full path to the entry if it does not exist already match full_path.parent() { @@ -158,3 +217,195 @@ impl TarZstdPackage { Self::unpack(file, into) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Cursor, Write}; + + fn tar_header(kind: tar::EntryType, path: &str, size: u64) -> tar::Header { + let mut header = tar::Header::new_gnu(); + header.set_entry_type(kind); + // Bypass `set_path`, which refuses to write invalid paths. + header.as_old_mut().name[..path.len()].copy_from_slice(path.as_bytes()); + header.set_mode(0o644); + header.set_size(size); + header.set_cksum(); + header + } + + enum TarEntry<'a> { + Dir(&'a str), + File(&'a str), + Symlink(&'a str, &'a str), + Hardlink(&'a str, &'a str), + } + + fn unpack_tar(entries: &[TarEntry<'_>]) -> (tempfile::TempDir, Result<()>) { + let mut builder = tar::Builder::new(Vec::new()); + for entry in entries { + match *entry { + TarEntry::Dir(path) => { + let mut header = tar_header(tar::EntryType::Directory, path, 0); + header.set_mode(0o755); + header.set_cksum(); + builder.append(&header, io::empty()).unwrap(); + } + TarEntry::File(path) => { + let header = tar_header(tar::EntryType::Regular, path, 2); + builder.append(&header, &b"hi"[..]).unwrap(); + } + TarEntry::Symlink(path, target) | TarEntry::Hardlink(path, target) => { + let kind = match *entry { + TarEntry::Symlink(..) => tar::EntryType::Symlink, + _ => tar::EntryType::Link, + }; + let mut header = tar_header(kind, path, 0); + header.set_link_name(target).unwrap(); + header.set_cksum(); + builder.append(&header, io::empty()).unwrap(); + } + } + } + let data = builder.into_inner().unwrap(); + let dir = tempfile::tempdir().unwrap(); + let result = TarPackage::unpack(&data[..], &dir.path().join("dest")); + (dir, result) + } + + fn unpack_zip(names: &[&str]) -> (tempfile::TempDir, Result<()>) { + let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new())); + for name in names { + zip.start_file(*name, zip::write::SimpleFileOptions::default()) + .unwrap(); + zip.write_all(b"hi").unwrap(); + } + let data = zip.finish().unwrap().into_inner(); + let dir = tempfile::tempdir().unwrap(); + let result = ZipPackage::unpack(Cursor::new(data), &dir.path().join("dest")); + (dir, result) + } + + #[test] + fn tar_unpacks_normal_archive() { + let (dir, result) = unpack_tar(&[ + TarEntry::Dir("pkg/"), + TarEntry::File("pkg/bin/lean"), + TarEntry::File("pkg/lib/libfoo.so.1"), + TarEntry::Symlink("pkg/lib/libfoo.so", "libfoo.so.1"), + ]); + result.unwrap(); + let dest = dir.path().join("dest"); + assert_eq!(fs::read(dest.join("bin/lean")).unwrap(), b"hi"); + assert_eq!(fs::read(dest.join("lib/libfoo.so")).unwrap(), b"hi"); + } + + #[test] + fn tar_rejects_parent_dir() { + let (dir, result) = unpack_tar(&[TarEntry::File("pkg/../evil")]); + assert!(result.is_err()); + assert!(!dir.path().join("evil").exists()); + } + + #[test] + fn tar_rejects_trailing_dot_or_space() { + for path in ["pkg/.. /evil", "pkg/... /evil", "pkg/evil."] { + let (_dir, result) = unpack_tar(&[TarEntry::File(path)]); + assert!(result.is_err(), "{}", path); + } + } + + #[test] + fn tar_strips_root_dir() { + // The leading `/` is the stripped first component, so the entry stays inside. + let (dir, result) = unpack_tar(&[TarEntry::File("/evil")]); + result.unwrap(); + assert!(dir.path().join("dest/evil").exists()); + } + + #[test] + fn tar_rejects_symlink_outside() { + for target in ["..", "../evil", "/tmp", "sub/../..", ".. "] { + let (_dir, result) = unpack_tar(&[TarEntry::Symlink("pkg/link", target)]); + assert!(result.is_err(), "{}", target); + } + } + + #[test] + fn tar_rejects_file_through_symlink_outside() { + let (dir, result) = unpack_tar(&[ + TarEntry::Symlink("pkg/link", ".."), + TarEntry::File("pkg/link/evil"), + ]); + assert!(result.is_err()); + assert!(!dir.path().join("evil").exists()); + } + + #[test] + fn tar_rejects_symlink_at_destination() { + for path in ["pkg", "pkg/."] { + let (dir, result) = + unpack_tar(&[TarEntry::Symlink(path, "."), TarEntry::File("pkg/evil")]); + // On Unix, `symlink` would also fail on the `dest/` path that results + // from joining the empty path, so check that our own check fires. + let err = result.unwrap_err().to_string(); + assert!(err.starts_with("invalid path in archive"), "{}: {}", path, err); + assert!(!dir.path().join("evil").exists(), "{}", path); + } + } + + #[test] + fn tar_rejects_file_at_destination() { + for path in ["pkg", "pkg/."] { + let (dir, result) = unpack_tar(&[TarEntry::File(path)]); + let err = result.unwrap_err().to_string(); + assert!(err.starts_with("invalid path in archive"), "{}: {}", path, err); + assert!(!dir.path().join("dest").is_file(), "{}", path); + } + } + + #[test] + fn tar_rejects_hard_link() { + // Hard link targets are not relative to the destination but to the + // working directory, which for tests contains `Cargo.toml`. + let (_dir, result) = unpack_tar(&[TarEntry::Hardlink("pkg/b", "Cargo.toml")]); + assert!(result.is_err()); + } + + #[test] + fn zip_unpacks_normal_archive() { + let (dir, result) = unpack_zip(&["pkg/bin/lean.exe", "pkg/lib/foo.dll"]); + result.unwrap(); + let dest = dir.path().join("dest"); + assert_eq!(fs::read(dest.join("bin/lean.exe")).unwrap(), b"hi"); + assert_eq!(fs::read(dest.join("lib/foo.dll")).unwrap(), b"hi"); + } + + #[test] + fn zip_rejects_parent_dir() { + for name in ["pkg/../evil", "pkg/sub/../../evil", "pkg/.. /evil"] { + let (dir, result) = unpack_zip(&[name]); + assert!(result.is_err(), "{}", name); + assert!(!dir.path().join("evil").exists()); + } + } + + #[test] + fn zip_rejects_file_at_destination() { + for name in ["pkg", "pkg/."] { + let (dir, result) = unpack_zip(&[name]); + let err = result.unwrap_err().to_string(); + assert!(err.starts_with("invalid path in archive"), "{}: {}", name, err); + assert!(!dir.path().join("dest").is_file(), "{}", name); + } + } + + #[cfg(windows)] + #[test] + fn zip_rejects_drive_prefix() { + for name in ["pkg/C:/evil", "pkg\\C:\\evil", "pkg/C:evil"] { + let (_dir, result) = unpack_zip(&[name]); + assert!(result.is_err(), "{}", name); + } + } +}