Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/archive/sevenz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ use crate::{
info,
list::{FileInArchive, ListFileType},
utils::{
BytesFmt, FileVisibilityPolicy, PathFmt, cd_into_same_dir_as, copy_limited_decompression,
BytesFmt, FileVisibilityPolicy, PathFmt, cd_into_same_dir_as, check_entry_path, copy_limited_decompression,
ensure_parent_dir_exists, is_same_file_as_output, resolve_extraction_conflict, validate_dest_inside_root,
validate_entry_path,
windows_unsafe_name_reason,
},
warning,
};
Expand All @@ -42,10 +42,10 @@ where
// Manually handle writing all files from 7z archive (the library defaults ignore empty files)

let name_as_path = Path::new(entry.name());
let safe_relpath = match validate_entry_path(name_as_path) {
let safe_relpath = match check_entry_path(name_as_path) {
Ok(p) => p,
Err(e) => {
warning!("skipping unsafe 7z entry {}: {}", PathFmt(name_as_path), e);
Err(reason) => {
warning!("skipping unsafe 7z entry {}: {}", PathFmt(name_as_path), reason);
return Ok(true);
}
};
Expand Down Expand Up @@ -183,6 +183,13 @@ where
let entry = entry?;
let path = entry.path();

// On Windows, opening a path that ends in a reserved DOS device
// name opens the device itself instead of the file; skip it.
if let Some(reason) = windows_unsafe_name_reason(path) {
warning!("skipping {}: {}", PathFmt(path), reason);
continue;
}

// Avoid compressing the output file into itself
if let Ok(handle) = output_handle.as_ref()
&& is_same_file_as_output(path, handle)
Expand Down
48 changes: 39 additions & 9 deletions src/archive/tar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ use crate::{
info,
list::{FileInArchive, ListFileType},
utils::{
self, BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, create_symlink, is_same_file_as_output,
read_file_type, resolve_extraction_conflict, sanitize_archive_mode, set_permission_mode,
validate_dest_inside_root, validate_entry_path, validate_symlink_target,
self, BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, check_entry_path, create_symlink,
is_same_file_as_output, read_file_type, resolve_extraction_conflict, sanitize_archive_mode,
set_permission_mode, validate_dest_inside_root, validate_symlink_target, windows_unsafe_name_reason,
},
warning,
};
Expand All @@ -33,6 +33,18 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path, question_policy:
let mut files_unpacked = 0;
let mut read_only_dirs_and_modes = Vec::new();

// Unsafe entry names are skipped with a warning, like in zip and 7z,
// instead of aborting the extraction and leaving partial output behind.
let checked_entry_path = |raw_path: &Path| -> Option<PathBuf> {
match check_entry_path(raw_path) {
Ok(path) => Some(path),
Err(reason) => {
warning!("skipping unsafe tar entry {}: {}", PathFmt(raw_path), reason);
None
}
}
};

for entry in archive.entries()? {
let mut entry = entry?;

Expand All @@ -42,7 +54,9 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path, question_policy:
match entry.header().entry_type() {
tar::EntryType::Symlink => {
let raw_path = entry.path()?.into_owned();
let safe_relpath = validate_entry_path(&raw_path)?;
let Some(safe_relpath) = checked_entry_path(&raw_path) else {
continue;
};
let full_path = output_folder.join(&safe_relpath);
let target = entry
.link_name()?
Expand All @@ -54,12 +68,16 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path, question_policy:
}
tar::EntryType::Link => {
let raw_link = entry.path()?.into_owned();
let safe_link_path = validate_entry_path(&raw_link)?;
let Some(safe_link_path) = checked_entry_path(&raw_link) else {
continue;
};
let raw_target = entry
.link_name()?
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "Missing hardlink target"))?
.into_owned();
let safe_target = validate_entry_path(&raw_target)?;
let Some(safe_target) = checked_entry_path(&raw_target) else {
continue;
};

let full_link_path = output_folder.join(&safe_link_path);
let full_target_path = output_folder.join(&safe_target);
Expand All @@ -70,7 +88,9 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path, question_policy:
}
tar::EntryType::Regular | tar::EntryType::GNUSparse => {
let raw_path = entry.path()?.into_owned();
let safe_relpath = validate_entry_path(&raw_path)?;
let Some(safe_relpath) = checked_entry_path(&raw_path) else {
continue;
};
let full_path = output_folder.join(&safe_relpath);

let Some(dest) = resolve_extraction_conflict(&full_path, question_policy)? else {
Expand All @@ -88,14 +108,17 @@ pub fn unpack_archive(reader: impl Read, output_folder: &Path, question_policy:
let original_mode = entry.header().mode()?;
let is_writeable = (original_mode & 0o200) != 0;

let raw_path = entry.path()?.into_owned();
let Some(safe_relpath) = checked_entry_path(&raw_path) else {
continue;
};

// this is no-op when dir already exists, errs if a file with another type is found there
entry.unpack_in(output_folder)?;

if cfg!(unix) && is_writeable.not() {
// 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 safe_relpath = validate_entry_path(&original_path)?;
let unpacked = output_folder.join(&safe_relpath);
set_permission_mode(&unpacked, sanitize_archive_mode(original_mode) | 0o200)?;

Expand Down Expand Up @@ -185,6 +208,13 @@ where
for entry in iter {
let path = entry?;

// On Windows, opening a path that ends in a reserved DOS device
// name opens the device itself instead of the file; skip it.
if let Some(reason) = windows_unsafe_name_reason(&path) {
warning!("skipping {}: {}", PathFmt(&path), reason);
continue;
}

// Avoid compressing the output file into itself
if let Ok(handle) = output_handle.as_ref()
&& is_same_file_as_output(&path, handle)
Expand Down
21 changes: 19 additions & 2 deletions src/archive/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ use crate::{
info, info_accessible,
list::{FileInArchive, ListFileType},
utils::{
BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, cd_into_same_dir_as,
BytesFmt, FileType, FileVisibilityPolicy, PathFmt, canonicalize, cd_into_same_dir_as, check_entry_path,
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, resolve_extraction_conflict,
strip_cur_dir, validate_dest_inside_root, validate_symlink_target,
strip_cur_dir, validate_dest_inside_root, validate_symlink_target, windows_unsafe_name_reason,
},
warning,
};
Expand Down Expand Up @@ -57,6 +57,16 @@ where
}
};

// `enclosed_name` strips traversal but not Windows-only pitfalls such as
// reserved DOS device names (NUL, COM1, ...) or ':' (NTFS ADS separator).
let relpath = match check_entry_path(&relpath) {
Ok(path) => path,
Err(reason) => {
warning!("skipping entry {} with unsafe name {}: {}", idx, file.name(), reason);
continue;
}
};

let file_path = output_folder.join(&relpath);

validate_dest_inside_root(output_folder, &file_path)?;
Expand Down Expand Up @@ -236,6 +246,13 @@ where
for entry in iter {
let path = entry?;

// On Windows, opening a path that ends in a reserved DOS device
// name opens the device itself instead of the file; skip it.
if let Some(reason) = windows_unsafe_name_reason(&path) {
warning!("skipping {}: {}", PathFmt(&path), reason);
continue;
}

// Avoid compressing the output file into itself
if let Ok(handle) = output_handle.as_ref()
&& is_same_file_as_output(&path, handle)
Expand Down
24 changes: 22 additions & 2 deletions src/commands/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ use super::warn_user_about_loading_sevenz_in_memory;
use crate::{
BUFFER_CAPACITY, QuestionAction, QuestionPolicy, Result, archive,
commands::warn_user_about_loading_zip_in_memory,
error::FinalError,
extension::{CompressionFormat::*, Extension, split_first_compression_format},
info_accessible,
utils::{
BytesFmt, FileVisibilityPolicy, file_size,
BytesFmt, FileVisibilityPolicy, PathFmt, file_size,
io::lock_and_flush_output_stdio,
threads::{logical_thread_count, physical_thread_count},
user_wants_to_continue,
user_wants_to_continue, windows_unsafe_name_reason,
},
};

Expand All @@ -41,6 +42,25 @@ pub fn compress_files(
file_visibility_policy: FileVisibilityPolicy,
level: Option<i16>,
) -> Result<bool> {
// On Windows an input named after a DOS device (NUL, COM1, ...) or
// containing ':' opens the device / an NTFS stream instead of the file.
for file in &files {
if let Some(reason) = windows_unsafe_name_reason(file) {
return Err(FinalError::with_title("Refusing to compress unsafe input path")
.detail(format!("input: {}", PathFmt(file)))
.detail(reason)
.into());
}
}

// Writing to such an output name hits the same devices / NTFS streams.
if let Some(reason) = windows_unsafe_name_reason(output_path) {
return Err(FinalError::with_title("Refusing to compress to unsafe output path")
.detail(format!("output: {}", PathFmt(output_path)))
.detail(reason)
.into());
}

// If the input files contain a directory, then the total size will be underestimated
let file_writer = BufWriter::with_capacity(BUFFER_CAPACITY, output_file);

Expand Down
9 changes: 9 additions & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ pub fn run(args: CliArgs, question_policy: QuestionPolicy, file_visibility_polic
)?;
check::check_archive_formats_position(&formats, &output_path)?;

// Refuse before the output file is created: on Windows a name like
// `NUL.tar` would otherwise open the device or an NTFS stream.
if let Some(reason) = utils::windows_unsafe_name_reason(&output_path) {
return Err(FinalError::with_title("Refusing to compress to unsafe output path")
.detail(format!("output: {}", utils::PathFmt(&output_path)))
.detail(reason)
.into());
}

let (output_file, output_path) = match utils::create_file_or_prompt_on_conflict(
&output_path,
question_policy,
Expand Down
Loading
Loading