diff --git a/CHANGELOG.md b/CHANGELOG.md index f5ccd596f..0929a40bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Categories Used: - Tweaks - anything that doesn't fit into other categories, small typo fixes, most CI stuff, meta changes (e.g. README updates), etc. - Removals - removal of a feature (and most likely a breaking change) +- Safety - Changes focused on preventing malicious attacks **Bullet points in chronological order by PR** @@ -21,6 +22,16 @@ Categories Used: ### Improvements +- Add `OUCH_PASSWORD` env var as alternative to `--password` (https://github.com/ouch-org/ouch/pull/951). +- Add `OUCH_MAX_DECOMPRESSED_BYTES` to optionally cap decompressed output size and listing (https://github.com/ouch-org/ouch/pull/951). + +### Safety + +- Limit LZMA/XZ decoder size limit to 256 MiB to prevent OOM attacks from malicious inputs (https://github.com/ouch-org/ouch/pull/951). +- Prevent path traversal and symlink/hardlink escape attacks in tar, zip, 7z, and rar entries (https://github.com/ouch-org/ouch/pull/951). +- Sanitize archive-controlled filenames, comments, symlink targets, and error text before printing to the terminal (https://github.com/ouch-org/ouch/pull/951). +- Strip special permission bits from archive-supplied file modes and create zip outputs with final sanitized permissions immediately (https://github.com/ouch-org/ouch/pull/951). + ### Bug Fixes - Decompress: exit with a non-zero code when a file conflict can't be resolved on non-interactive stdin, instead of silently skipping and reporting success (https://github.com/ouch-org/ouch/pull/1011) @@ -49,9 +60,6 @@ Categories Used: - Fix various panics not handled gracefully (https://github.com/ouch-org/ouch/pull/950) - Handle GNUSparse archive entries during tar decompression (https://github.com/ouch-org/ouch/pull/975) -### Tweaks - - ## [0.7.1](https://github.com/ouch-org/ouch/releases/tag/0.7.1) ### Bug Fixes diff --git a/src/archive/rar.rs b/src/archive/rar.rs index acc55575b..d8e2c1c3a 100644 --- a/src/archive/rar.rs +++ b/src/archive/rar.rs @@ -11,7 +11,8 @@ use crate::{ error::{Error, FinalError, Result}, info, list::{FileInArchive, ListFileType}, - utils::{BytesFmt, PathFmt}, + utils::{BytesFmt, PathFmt, validate_entry_path}, + warning, }; /// Unpacks the archive given by `archive_path` into the folder given by `output_folder`. @@ -26,8 +27,18 @@ pub fn unpack_archive(archive_path: &Path, output_folder: &Path, password: Optio let mut files_unpacked: u64 = 0; let mut first_err: Option<(PathBuf, i32)> = None; + let mut unsafe_path: Option<(PathBuf, String)> = None; let cb_result = archive.extract_all_with_callback(output_folder, |event| match event { + ExtractEvent::Start { filename, .. } => { + if let Err(e) = validate_entry_path(&filename) { + warning!("refusing unsafe rar entry {}: {}", PathFmt(&filename), e); + unsafe_path = Some((filename, e.to_string())); + false + } else { + true + } + } ExtractEvent::Ok { filename, size } => { info!("extracted ({}) {}", BytesFmt(size), PathFmt(&filename)); files_unpacked += 1; @@ -55,6 +66,13 @@ pub fn unpack_archive(archive_path: &Path, output_folder: &Path, password: Optio _ => true, }); + if let Some((path, reason)) = unsafe_path { + return Err(Error::Custom { + reason: FinalError::with_title(format!("refusing to extract unsafe rar entry {}", PathFmt(&path))) + .detail(reason), + }); + } + if let Some((path, code)) = first_err { let inner = UnrarError::from(Code::from(code), When::Process).to_string(); return Err(Error::Custom { diff --git a/src/archive/sevenz.rs b/src/archive/sevenz.rs index a45248e61..5e1b33b40 100644 --- a/src/archive/sevenz.rs +++ b/src/archive/sevenz.rs @@ -1,8 +1,7 @@ //! SevenZip archive format compress function use std::{ - env, - io::{self, BufWriter, Read, Seek, Write}, + io::{BufWriter, Read, Seek, Write}, path::{Path, PathBuf}, }; @@ -18,7 +17,8 @@ use crate::{ info, list::{FileInArchive, ListFileType}, utils::{ - BytesFmt, FileVisibilityPolicy, PathFmt, cd_into_same_dir_as, ensure_parent_dir_exists, is_same_file_as_output, + BytesFmt, FileVisibilityPolicy, PathFmt, cd_into_same_dir_as, copy_limited_decompression, + ensure_parent_dir_exists, is_same_file_as_output, validate_dest_inside_root, validate_entry_path, }, warning, }; @@ -33,7 +33,20 @@ where |entry: &ArchiveEntry, reader: &mut dyn Read, path: &PathBuf| -> Result { // Manually handle writing all files from 7z archive (the library defaults ignore empty files) - let file_path = output_path.join(entry.name()); + let name_as_path = Path::new(entry.name()); + let safe_relpath = match validate_entry_path(name_as_path) { + Ok(p) => p, + Err(e) => { + warning!("skipping unsafe 7z entry {}: {}", PathFmt(name_as_path), e); + return Ok(true); + } + }; + let file_path = output_path.join(&safe_relpath); + + if let Err(e) = validate_dest_inside_root(output_path, &file_path) { + warning!("skipping 7z entry {}: {}", PathFmt(&file_path), e); + return Ok(true); + } if entry.is_directory() { info!("File {} extracted to {}", entry.name(), PathFmt(&file_path)); @@ -47,7 +60,7 @@ where let file = fs::File::create(path)?; let mut writer = BufWriter::new(file); - io::copy(reader, &mut writer)?; + copy_limited_decompression(reader, &mut writer)?; use filetime_creation as ft; // Surface mtime-set failures as warnings so users know timestamps weren't preserved @@ -136,6 +149,7 @@ where for filename in files { let previous_location = cd_into_same_dir_as(filename)?; + let _cwd_guard = crate::utils::CwdGuard::new(previous_location); // Unwrap safety: // paths should be canonicalized by now, and the root directory rejected. @@ -172,8 +186,6 @@ where writer.push_archive_entry::(entry, entry_data)?; } - - env::set_current_dir(previous_location)?; } let bytes = writer.finish()?; diff --git a/src/archive/tar.rs b/src/archive/tar.rs index 843cd3cf7..fbd8dfa4e 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -4,7 +4,6 @@ use std::os::unix::fs::MetadataExt; use std::{ collections::HashMap, - env, io::{self, prelude::*}, ops::Not, path::{Path, PathBuf}, @@ -20,7 +19,8 @@ use crate::{ list::{FileInArchive, ListFileType}, utils::{ self, BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, create_symlink, is_same_file_as_output, - read_file_type, set_permission_mode, + read_file_type, sanitize_archive_mode, set_permission_mode, validate_dest_inside_root, validate_entry_path, + validate_symlink_target, }, warning, }; @@ -38,23 +38,31 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result { match entry.header().entry_type() { tar::EntryType::Symlink => { - let relative_path = entry.path()?; - let full_path = output_folder.join(&relative_path); + let raw_path = entry.path()?.into_owned(); + let safe_relpath = validate_entry_path(&raw_path)?; + let full_path = output_folder.join(&safe_relpath); let target = entry .link_name()? .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Missing symlink target"))?; + validate_symlink_target(&safe_relpath, &target)?; + validate_dest_inside_root(output_folder, &full_path)?; create_symlink(&target, &full_path)?; } tar::EntryType::Link => { - let link_path = entry.path()?; - let target = entry + let raw_link = entry.path()?.into_owned(); + let safe_link_path = validate_entry_path(&raw_link)?; + let raw_target = entry .link_name()? - .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Missing hardlink target"))?; + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Missing hardlink target"))? + .into_owned(); + let safe_target = validate_entry_path(&raw_target)?; - let full_link_path = output_folder.join(&link_path); - let full_target_path = output_folder.join(&target); + let full_link_path = output_folder.join(&safe_link_path); + let full_target_path = output_folder.join(&safe_target); + validate_dest_inside_root(output_folder, &full_link_path)?; + validate_dest_inside_root(output_folder, &full_target_path)?; fs::hard_link(&full_target_path, &full_link_path)?; } tar::EntryType::Regular | tar::EntryType::GNUSparse => { @@ -71,10 +79,11 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path) -> Result { // We unpacked a read-only directory, make it writeable so that we can // create the files inside of it, by the end, restore the original mode let original_path = entry.path()?.to_path_buf(); - let unpacked = output_folder.join(&original_path); - set_permission_mode(&unpacked, original_mode | 0o200)?; + let safe_relpath = validate_entry_path(&original_path)?; + let unpacked = output_folder.join(&safe_relpath); + set_permission_mode(&unpacked, sanitize_archive_mode(original_mode) | 0o200)?; - read_only_dirs_and_modes.push((original_path, original_mode)); + read_only_dirs_and_modes.push((original_path, sanitize_archive_mode(original_mode))); } } _ => continue, @@ -146,6 +155,7 @@ where for explicit_path in explicit_paths { let previous_location = utils::cd_into_same_dir_as(explicit_path)?; + let _cwd_guard = utils::CwdGuard::new(previous_location); // Unwrap expectation: // paths should be canonicalized by now, and the root directory rejected. @@ -231,7 +241,6 @@ where } } } - env::set_current_dir(previous_location)?; } Ok(builder.into_inner()?) diff --git a/src/archive/zip.rs b/src/archive/zip.rs index eae9d327a..d8ab5e0e2 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -3,7 +3,6 @@ #[cfg(unix)] use std::os::unix::fs::PermissionsExt; use std::{ - env, io::{self, prelude::*}, path::{Path, PathBuf}, }; @@ -15,15 +14,18 @@ use same_file::Handle; use time::{OffsetDateTime, PrimitiveDateTime}; use zip::{self, DateTime, ZipArchive, read::ZipFile}; +#[cfg(unix)] +use crate::utils::sanitize_archive_mode; use crate::{ Result, error::FinalError, info, info_accessible, list::{FileInArchive, ListFileType}, utils::{ - BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, cd_into_same_dir_as, create_symlink, - ensure_parent_dir_exists, get_invalid_utf8_paths, is_same_file_as_output, pretty_format_list_of_paths, - read_file_type, strip_cur_dir, + BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, cd_into_same_dir_as, + copy_limited_decompression, create_symlink, ensure_parent_dir_exists, get_invalid_utf8_paths, + is_same_file_as_output, pretty_format_list_of_paths, read_file_type, strip_cur_dir, validate_dest_inside_root, + validate_symlink_target, }, warning, }; @@ -42,12 +44,17 @@ where Some(password) => archive.by_index_decrypt(idx, password)?, None => archive.by_index(idx)?, }; - let file_path = match file.enclosed_name() { + let relpath = match file.enclosed_name() { Some(path) => path.to_owned(), - None => continue, + None => { + warning!("skipping entry {} with unsafe name: {}", idx, file.name()); + continue; + } }; - let file_path = output_folder.join(file_path); + let file_path = output_folder.join(&relpath); + + validate_dest_inside_root(output_folder, &file_path)?; display_zip_comment_if_exists(&file); @@ -62,6 +69,7 @@ where let mut target = String::new(); file.read_to_string(&mut target)?; + validate_symlink_target(&relpath, Path::new(&target))?; #[cfg(unix)] std::os::unix::fs::symlink(&target, &file_path)?; #[cfg(windows)] @@ -81,12 +89,27 @@ where let mut target = String::new(); file.read_to_string(&mut target)?; + validate_symlink_target(&relpath, Path::new(&target))?; info!("linking {} -> \"{}\"", PathFmt(file_path), target); create_symlink(Path::new(&target), file_path)?; } else { + #[cfg(unix)] + let mut output_file = { + use fs_err::os::unix::fs::OpenOptionsExt; + let mode = file.unix_mode().map(sanitize_archive_mode).unwrap_or(0o644); + fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(mode) + .open(file_path)? + }; + #[cfg(not(unix))] let mut output_file = fs::File::create(file_path)?; - io::copy(&mut file, &mut output_file)?; + { + copy_limited_decompression(&mut file, &mut output_file)?; + } set_last_modified_time(&file, file_path)?; #[cfg(unix)] unix_set_permissions(file_path, &file)?; @@ -182,6 +205,7 @@ where for explicit_path in input_filenames { let previous_location = cd_into_same_dir_as(explicit_path)?; + let _cwd_guard = crate::utils::CwdGuard::new(previous_location); // Unwrap safety: // paths should be canonicalized by now, and the root directory rejected. @@ -255,8 +279,6 @@ where } } } - - env::set_current_dir(previous_location)?; } let bytes = writer.finish()?; @@ -315,7 +337,7 @@ fn unix_set_permissions(file_path: &Path, file: &ZipFile<'_, R>) -> Res use std::fs::Permissions; if let Some(mode) = file.unix_mode() { - fs::set_permissions(file_path, Permissions::from_mode(mode))?; + fs::set_permissions(file_path, Permissions::from_mode(sanitize_archive_mode(mode)))?; } Ok(()) diff --git a/src/cli/args.rs b/src/cli/args.rs index 0da80e4f8..5d9973472 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -43,7 +43,7 @@ pub struct CliArgs { pub format: Option, /// Decompress or list with password - #[arg(short, long = "password", aliases = ["pass", "pw"], global = true)] + #[arg(short, long = "password", aliases = ["pass", "pw"], env = "OUCH_PASSWORD", global = true)] pub password: Option, /// Concurrent working threads diff --git a/src/commands/decompress.rs b/src/commands/decompress.rs index 274ec01a7..dae6396fc 100644 --- a/src/commands/decompress.rs +++ b/src/commands/decompress.rs @@ -16,7 +16,7 @@ use crate::{ info, info_accessible, non_archive::lz4::MultiFrameLz4Decoder, utils::{ - self, BytesFmt, PathFmt, file_size, + self, BytesFmt, LZMA_MEMLIMIT_BYTES, LimitedReader, PathFmt, copy_limited_decompression, file_size, io::{ReadSeek, lock_and_flush_output_stdio}, is_path_stdin, resolve_path_conflict, user_wants_to_continue, }, @@ -65,7 +65,11 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { Box::new(bzip3::read::Bz3Decoder::new(decoder)?) } Lz4 => Box::new(MultiFrameLz4Decoder::new(decoder)), - Lzma => Box::new(lzma_rust2::LzmaReader::new_mem_limit(decoder, u32::MAX, None)?), + Lzma => Box::new(lzma_rust2::LzmaReader::new_mem_limit( + decoder, + LZMA_MEMLIMIT_BYTES, + None, + )?), Xz => Box::new(lzma_rust2::XzReader::new(decoder, true)), Lzip => Box::new(lzma_rust2::LzipReader::new(decoder)), Snappy => Box::new(snap::read::FrameDecoder::new(decoder)), @@ -108,7 +112,9 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { let control_flow = match first_extension { Gzip | Bzip | Bzip3 | Lz4 | Lzma | Xz | Lzip | Snappy | Zstd | Brotli => { let reader = create_decoder_up_to_first_extension()?; - let mut reader = chain_reader_decoder(&first_extension, reader)?; + let reader = chain_reader_decoder(&first_extension, reader)?; + // Bomb cap: abort if decompressed output exceeds OUCH_MAX_DECOMPRESSED_BYTES + let mut reader = LimitedReader::new(reader); let (mut writer, final_output_path) = match utils::create_file_or_prompt_on_conflict( &options.output_file_path, @@ -125,7 +131,10 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { }) } Tar => unpack_archive( - |output_dir| crate::archive::tar::unpack_archive(create_decoder_up_to_first_extension()?, output_dir), + |output_dir| { + let reader = LimitedReader::new(create_decoder_up_to_first_extension()?); + crate::archive::tar::unpack_archive(reader, output_dir) + }, archive_output_dir, options.question_policy, )?, @@ -160,7 +169,8 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { drop(locks); let mut vec = vec![]; - io::copy(&mut create_decoder_up_to_first_extension()?, &mut vec)?; + // Bomb cap: abort if the in-memory decompressed image exceeds the limit + copy_limited_decompression(create_decoder_up_to_first_extension()?, &mut vec)?; Box::new(io::Cursor::new(vec)) } else { Box::new(BufReader::with_capacity( @@ -179,7 +189,7 @@ pub fn decompress_file(options: DecompressOptions) -> Result<()> { Rar => { let unpack_fn: Box Result> = if options.formats.len() > 1 || input_is_stdin { let mut temp_file = tempfile::NamedTempFile::new()?; - io::copy(&mut create_decoder_up_to_first_extension()?, &mut temp_file)?; + copy_limited_decompression(create_decoder_up_to_first_extension()?, &mut temp_file)?; Box::new(move |output_dir| { crate::archive::rar::unpack_archive(temp_file.path(), output_dir, options.password) }) diff --git a/src/commands/list.rs b/src/commands/list.rs index 78c9a79fe..7afc49932 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -11,7 +11,10 @@ use crate::{ extension::CompressionFormat::{self, *}, list::{self, FileInArchive, ListOptions}, non_archive::lz4::MultiFrameLz4Decoder, - utils::{io::lock_and_flush_output_stdio, user_wants_to_continue}, + utils::{ + LZMA_MEMLIMIT_BYTES, LimitedReader, copy_limited_decompression, io::lock_and_flush_output_stdio, + user_wants_to_continue, + }, }; /// File at archive_path is opened for reading, example: "archive.tar.gz" @@ -57,7 +60,11 @@ pub fn list_archive_contents( Box::new(bzip3::read::Bz3Decoder::new(decoder)?) } Lz4 => Box::new(MultiFrameLz4Decoder::new(decoder)), - Lzma => Box::new(lzma_rust2::LzmaReader::new_mem_limit(decoder, u32::MAX, None)?), + Lzma => Box::new(lzma_rust2::LzmaReader::new_mem_limit( + decoder, + LZMA_MEMLIMIT_BYTES, + None, + )?), Xz => Box::new(lzma_rust2::XzReader::new(decoder, true)), Lzip => Box::new(lzma_rust2::LzipReader::new(decoder)), Snappy => Box::new(snap::read::FrameDecoder::new(decoder)), @@ -79,7 +86,10 @@ pub fn list_archive_contents( let archive_format = misplaced_archive_format.unwrap_or(formats[0]); let files: Box>> = match archive_format { - Tar => Box::new(crate::archive::tar::list_archive(tar::Archive::new(reader))?), + Tar => { + let limited = LimitedReader::new(reader); + Box::new(crate::archive::tar::list_archive(tar::Archive::new(limited))?) + } Zip => { if formats.len() > 1 { // Make thread own locks to keep output messages adjacent @@ -92,7 +102,8 @@ pub fn list_archive_contents( } let mut vec = vec![]; - io::copy(&mut reader, &mut vec)?; + // Bomb cap: abort if the in-memory decompressed image exceeds the limit + copy_limited_decompression(&mut reader, &mut vec)?; let zip_archive = zip::ZipArchive::new(io::Cursor::new(vec))?; Box::new(crate::archive::zip::list_archive(zip_archive, password)) @@ -101,7 +112,8 @@ pub fn list_archive_contents( Rar => { if formats.len() > 1 { let mut temp_file = tempfile::NamedTempFile::new()?; - io::copy(&mut reader, &mut temp_file)?; + // Bomb cap: abort if decompressed output to tempfile exceeds the limit + copy_limited_decompression(&mut reader, &mut temp_file)?; Box::new(crate::archive::rar::list_archive(temp_file.path(), password)?) } else { Box::new(crate::archive::rar::list_archive(archive_path, password)?) @@ -122,7 +134,8 @@ pub fn list_archive_contents( drop(locks); let mut vec = vec![]; - io::copy(&mut reader, &mut vec)?; + // Bomb cap: abort if the in-memory decompressed image exceeds the limit + copy_limited_decompression(&mut reader, &mut vec)?; Box::new(archive::sevenz::list_archive(io::Cursor::new(vec), password)?) } else { diff --git a/src/error.rs b/src/error.rs index 29e96dd9a..c1757dbc2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -11,6 +11,7 @@ use std::{ use crate::{ accessible::is_running_in_accessible_mode, extension::{PRETTY_SUPPORTED_ALIASES, PRETTY_SUPPORTED_EXTENSIONS}, + utils::sanitize_for_display, }; /// All errors that can be generated by `ouch` @@ -88,14 +89,14 @@ impl Display for FinalError { // // When in ACCESSIBLE mode, the square brackets are suppressed if is_running_in_accessible_mode() { - write!(f, "{}ERROR{}: {}", *RED, *RESET, self.title)?; + write!(f, "{}ERROR{}: {}", *RED, *RESET, sanitize_for_display(&self.title))?; } else { - write!(f, "{}[ERROR]{} {}", *RED, *RESET, self.title)?; + write!(f, "{}[ERROR]{} {}", *RED, *RESET, sanitize_for_display(&self.title))?; } // Details for detail in &self.details { - write!(f, "\n - {}{}{}", *YELLOW, detail, *RESET)?; + write!(f, "\n - {}{}{}", *YELLOW, sanitize_for_display(detail), *RESET)?; } // Hints @@ -107,11 +108,11 @@ impl Display for FinalError { if is_running_in_accessible_mode() { write!(f, "\n{}hints:{}", *GREEN, *RESET)?; for hint in &self.hints { - write!(f, "\n{hint}")?; + write!(f, "\n{}", sanitize_for_display(hint))?; } } else { for hint in &self.hints { - write!(f, "\n{}hint:{} {}", *GREEN, *RESET, hint)?; + write!(f, "\n{}hint:{} {}", *GREEN, *RESET, sanitize_for_display(hint))?; } } } @@ -275,3 +276,24 @@ impl From for Error { Self::Custom { reason: err } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn final_error_display_sanitizes_untrusted_text() { + let err = FinalError::with_title("bad\u{1b}]title") + .detail("bad\u{202e}detail") + .hint("bad\u{200b}hint"); + + let rendered = err.to_string(); + + assert!(!rendered.contains("\u{1b}]")); + assert!(!rendered.contains('\u{202e}')); + assert!(!rendered.contains('\u{200b}')); + assert!(rendered.contains("bad�]title")); + assert!(rendered.contains("bad�detail")); + assert!(rendered.contains("bad�hint")); + } +} diff --git a/src/list.rs b/src/list.rs index fb7fe5ada..b42c215e6 100644 --- a/src/list.rs +++ b/src/list.rs @@ -7,7 +7,11 @@ use std::{ }; use self::tree::Tree; -use crate::{Result, accessible::is_running_in_accessible_mode, utils::PathFmt}; +use crate::{ + Result, + accessible::is_running_in_accessible_mode, + utils::{NoQuotePathFmt, PathFmt}, +}; /// Options controlling how archive contents should be listed #[derive(Debug, Clone, Copy)] @@ -56,7 +60,7 @@ pub fn list_files( } else { for file in files { let FileInArchive { path, file_type } = file?; - print_entry(&mut out, path.display(), &file_type, list_options.quiet); + print_entry(&mut out, NoQuotePathFmt(&path), &file_type, list_options.quiet); } } Ok(()) @@ -86,14 +90,14 @@ fn print_entry(out: &mut impl Write, name: impl fmt::Display, file_type: &ListFi }; if is_running_in_accessible_mode() { - let _ = writeln!(out, "{name} -> {}{suffix}", target.display()); + let _ = writeln!(out, "{name} -> {}{suffix}", NoQuotePathFmt(target)); } else { let _ = writeln!( out, "{c}{name}{r} {c}-> {c}{target}{suffix}{r}", c = *CYAN, r = *ALL_RESET, - target = target.display() + target = NoQuotePathFmt(target) ); } } @@ -125,11 +129,10 @@ mod tree { path, }; - use bstr::{ByteSlice, ByteVec}; use indexmap::IndexMap; use super::{FileInArchive, ListFileType}; - use crate::{utils::NoQuotePathFmt, warning}; + use crate::warning; /// Directory tree #[derive(Debug, Default)] @@ -165,7 +168,7 @@ mod tree { Some(file) => { warning!( "multiple files with the same name in a single directory ({})", - NoQuotePathFmt(&file.path), + super::NoQuotePathFmt(&file.path), ); } } @@ -195,7 +198,7 @@ mod tree { }; super::print_entry( out, - as ByteVec>::from_os_str_lossy(name).as_bstr(), + super::NoQuotePathFmt(name.as_ref()), &file_type, false, // Always show targets in tree view, regardless of --quiet flag ); diff --git a/src/utils/formatting.rs b/src/utils/formatting.rs index 19461e7cb..5fed55069 100644 --- a/src/utils/formatting.rs +++ b/src/utils/formatting.rs @@ -99,6 +99,41 @@ pub fn append_ascii_suffix_to_os_str(os_str: &OsStr, ascii_suffix: &str) -> OsSt pub struct PathFmt<'a>(pub &'a Path); pub struct NoQuotePathFmt<'a>(pub &'a Path); +/// Returns true for bytes/chars that must be stripped before hitting a terminal +/// Covers C0 controls, DEL, C1 controls (U+0080..U+009F), line/paragraph separators, +/// BiDi formatting characters (Trojan Source, CVE-2021-42574), and zero-width +/// / invisible characters used for homoglyph or hidden-content attacks +pub fn is_unsafe_display_char(ch: char) -> bool { + match ch { + c if c.is_control() => true, + '\u{2028}' | '\u{2029}' => true, + '\u{202A}'..='\u{202E}' => true, + '\u{2066}'..='\u{2069}' => true, + '\u{200B}' | '\u{200C}' | '\u{200D}' | '\u{FEFF}' | '\u{00AD}' | '\u{180E}' | '\u{034F}' | '\u{061C}' => true, + _ => false, + } +} + +pub fn contains_unsafe_display_char(text: &str) -> bool { + text.chars().any(is_unsafe_display_char) +} + +pub fn sanitize_for_display(text: &str) -> Cow<'_, str> { + if !contains_unsafe_display_char(text) { + return Cow::Borrowed(text); + } + + let mut sanitized = String::with_capacity(text.len()); + for ch in text.chars() { + if is_unsafe_display_char(ch) { + sanitized.push('\u{FFFD}'); + } else { + sanitized.push(ch); + } + } + Cow::Owned(sanitized) +} + /// Same as NoQuotePathFmt, but surrounded by "". impl<'a> fmt::Display for PathFmt<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -107,6 +142,7 @@ impl<'a> fmt::Display for PathFmt<'a> { } /// Same as `path.display()` but try to strip some common noise prefixes +/// Also strips control bytes and BiDi/zero-width chars to prevent terminal injection impl<'a> fmt::Display for NoQuotePathFmt<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let path = self.0; @@ -122,7 +158,7 @@ impl<'a> fmt::Display for NoQuotePathFmt<'a> { path }; - write!(f, "{}", path.display()) + f.write_str(&sanitize_for_display(&path.display().to_string())) } } diff --git a/src/utils/fs.rs b/src/utils/fs.rs index b9df5e219..aaa882710 100644 --- a/src/utils/fs.rs +++ b/src/utils/fs.rs @@ -3,7 +3,7 @@ use std::{ borrow::Cow, env, - io::{self, Read}, + io::{self, Read, Write}, path::{Path, PathBuf}, }; @@ -118,6 +118,144 @@ pub fn create_dir_if_non_existent(path: &Path) -> Result<()> { } /// Ensures the parent directory of a file path exists, creating it if necessary. +/// Lexically normalize a relative path; returns None on absolute or escape via `..`. +pub fn normalize_safe_path(path: &Path) -> Option { + let mut out = PathBuf::new(); + for comp in path.components() { + match comp { + std::path::Component::Normal(c) => out.push(c), + std::path::Component::CurDir => {} + std::path::Component::ParentDir => { + if !out.pop() { + return None; + } + } + std::path::Component::Prefix(_) | std::path::Component::RootDir => return None, + } + } + Some(out) +} + +/// Reject ZipSlip-style entry paths; returns the lexically-normalized safe form. +pub fn validate_entry_path(path: &Path) -> Result { + normalize_safe_path(path).ok_or_else(|| { + FinalError::with_title("refusing to extract archive entry with unsafe path") + .detail(format!("entry: {}", PathFmt(path))) + .into() + }) +} + +/// Reject symlink targets whose relative path would escape the extraction root. +pub fn validate_symlink_target(link_relpath: &Path, target: &Path) -> Result<()> { + if target.is_absolute() { + return Ok(()); + } + let parent = link_relpath.parent().unwrap_or(Path::new("")); + if normalize_safe_path(&parent.join(target)).is_none() { + return Err( + FinalError::with_title("refusing to create symlink escaping extraction root") + .detail(format!("link: {} target: {}", PathFmt(link_relpath), PathFmt(target))) + .into(), + ); + } + Ok(()) +} + +/// Refuse to write through an on-disk symlink created by an earlier entry. +/// Walks every existing prefix between root and dest and errors if any component is a symlink. +pub fn validate_dest_inside_root(root: &Path, dest: &Path) -> Result<()> { + let rel = dest.strip_prefix(root).map_err(|_| { + FinalError::with_title("refusing to write outside extraction root").detail(format!("dest: {}", PathFmt(dest))) + })?; + let mut probe = root.to_path_buf(); + for comp in rel.components() { + probe.push(comp); + match fs::symlink_metadata(&probe) { + Ok(md) if md.file_type().is_symlink() => { + return Err( + FinalError::with_title("refusing to traverse on-disk symlink during extraction") + .detail(format!("path: {}", PathFmt(&probe))) + .into(), + ); + } + _ => {} + } + } + Ok(()) +} + +/// LZMA/XZ dictionary memory cap (256 MiB). Bounds malformed-stream allocations +pub const LZMA_MEMLIMIT_BYTES: u32 = 256 * 1024 * 1024; + +pub fn max_decompressed_bytes() -> u64 { + env::var("OUCH_MAX_DECOMPRESSED_BYTES") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(u64::MAX) +} + +pub struct LimitedReader { + inner: R, + remaining: u64, +} + +impl LimitedReader { + pub fn new(inner: R) -> Self { + Self { + inner, + remaining: max_decompressed_bytes(), + } + } +} + +pub fn copy_limited_decompression(reader: R, writer: &mut W) -> io::Result { + let mut limited = LimitedReader::new(reader); + io::copy(&mut limited, writer) +} + +impl Read for LimitedReader { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + if buf.is_empty() { + return Ok(0); + } + + if self.remaining == 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "decompression output exceeded configured limit (see OUCH_MAX_DECOMPRESSED_BYTES)", + )); + } + + let bytes_to_ready = usize::try_from(self.remaining).unwrap_or(usize::MAX).min(buf.len()); + let bytes_read = self.inner.read(&mut buf[..bytes_to_ready])?; + + self.remaining = self.remaining.saturating_sub(bytes_read as u64); + Ok(bytes_read) + } +} + +/// Strip setuid/setgid/sticky bits from an archive-supplied mode. +pub fn sanitize_archive_mode(mode: u32) -> u32 { + mode & 0o0777 +} + +/// RAII guard that restores the process CWD on drop. +pub struct CwdGuard(Option); + +impl CwdGuard { + pub fn new(previous: PathBuf) -> Self { + Self(Some(previous)) + } +} + +impl Drop for CwdGuard { + fn drop(&mut self) { + if let Some(p) = self.0.take() { + let _ = env::set_current_dir(&p); + } + } +} + pub fn ensure_parent_dir_exists(file_path: &Path) -> io::Result<()> { if let Some(parent) = file_path.parent() && !parent.fs_err_try_exists()? diff --git a/src/utils/io.rs b/src/utils/io.rs index 73028aff7..59fb20a94 100644 --- a/src/utils/io.rs +++ b/src/utils/io.rs @@ -1,5 +1,6 @@ use std::io::{self, StderrLock, StdoutLock, Write, stderr, stdout}; +#[cfg(unix)] use fs_err as fs; use crate::utils::logger; diff --git a/src/utils/logger.rs b/src/utils/logger.rs index dc5aec643..ac2ae37f6 100644 --- a/src/utils/logger.rs +++ b/src/utils/logger.rs @@ -6,7 +6,10 @@ use std::{ pub use logger_thread::spawn_logger_thread; -use super::colors::{GREEN, ORANGE, RESET}; +use super::{ + colors::{GREEN, ORANGE, RESET}, + formatting::{contains_unsafe_display_char, sanitize_for_display}, +}; use crate::accessible::is_running_in_accessible_mode; #[macro_export] @@ -261,7 +264,7 @@ mod logger_thread { LoggerCommand::Print(msg) => { // Append message to buffer if msg.should_display() { - writeln!(buffer, "{msg}").unwrap(); + write_message_to_buffer(&mut buffer, &msg); } } LoggerCommand::Flush { finished_barrier } => { @@ -277,11 +280,23 @@ mod logger_thread { } } + fn write_message_to_buffer(buffer: &mut Vec, msg: &PrintMessage) { + if !contains_unsafe_display_char(&msg.contents) { + writeln!(buffer, "{msg}").unwrap(); + } else { + let msg = PrintMessage { + contents: sanitize_for_display(&msg.contents).into_owned(), + accessible: msg.accessible, + level: msg.level, + }; + writeln!(buffer, "{msg}").unwrap(); + } + } + fn flush_logs_to_stderr(buffer: &mut Vec) { if !buffer.is_empty() { - if let Err(err) = stderr().write_all(buffer) { - panic!("Failed to write to STDERR: {err}"); - } + // Ignore stderr write failures (broken pipe, closed terminal) instead of panicking + let _ = stderr().write_all(buffer); buffer.clear(); } } diff --git a/tests/integration.rs b/tests/integration.rs index 2a2b8f7c6..4a8bb9eed 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -377,7 +377,7 @@ fn multiple_files_with_conflict_and_choice_to_rename_with_already_a_renamed( #[cfg(feature = "unrar")] #[test] fn unpack_rar() -> Result<(), Box> { - fn test_unpack_rar_single(input: &std::path::Path) -> Result<(), Box> { + fn test_unpack_rar_single(input: &Path) -> Result<(), Box> { let (_tempdir, dirpath) = testdir()?; let unpacked_path = &dirpath.join("testfile.txt"); ouch!("-A", "d", input, "-d", dirpath); @@ -399,7 +399,7 @@ fn unpack_rar() -> Result<(), Box> { #[cfg(feature = "unrar")] #[test] fn unpack_rar_stdin() -> Result<(), Box> { - fn test_unpack_rar_single(input: &std::path::Path, format: &str) -> Result<(), Box> { + fn test_unpack_rar_single(input: &Path, format: &str) -> Result<(), Box> { let (_tempdir, dirpath) = testdir()?; let unpacked_path = &dirpath.join("testfile.txt"); crate::utils::cargo_bin() diff --git a/tests/snapshots/ui__ui_test_usage_help_flag-2.snap b/tests/snapshots/ui__ui_test_usage_help_flag-2.snap index af5c50c0b..200e4da01 100644 --- a/tests/snapshots/ui__ui_test_usage_help_flag-2.snap +++ b/tests/snapshots/ui__ui_test_usage_help_flag-2.snap @@ -1,5 +1,6 @@ --- source: tests/ui.rs +assertion_line: 192 expression: "output_to_string(ouch!(\"-h\"))" --- A command-line utility for easily compressing and decompressing files and directories. @@ -20,7 +21,7 @@ Options: -q, --quiet Silence output -g, --gitignore Ignore files matched by git's ignore files -f, --format Specify the format of the archive - -p, --password Decompress or list with password + -p, --password Decompress or list with password [env: OUCH_PASSWORD=] -c, --threads Concurrent working threads -h, --help Print help (see more with '--help') -V, --version Print version diff --git a/tests/snapshots/ui__ui_test_usage_help_flag.snap b/tests/snapshots/ui__ui_test_usage_help_flag.snap index 7b0572a13..78b62fcf7 100644 --- a/tests/snapshots/ui__ui_test_usage_help_flag.snap +++ b/tests/snapshots/ui__ui_test_usage_help_flag.snap @@ -42,6 +42,8 @@ Options: -p, --password Decompress or list with password + + [env: OUCH_PASSWORD=] -c, --threads Concurrent working threads