diff --git a/cli/src/args.rs b/cli/src/args.rs index bced09b88..c00b25f86 100644 --- a/cli/src/args.rs +++ b/cli/src/args.rs @@ -275,13 +275,16 @@ processing all flags. --large-file [true|false] Whether to enable large file support. This may take up more space for records, but allows files over 32 bits in length to be - written, up to 64 bit sizes. + written, up to 64 bit sizes. File arguments over 32 bits in length (either provided + explicitly or encountered when traversing a recursive directory) will have this flag set + automatically, without affecting the sticky value for later options. Therefore, this + option likely never has to be set explicitly by the user. Non-sticky attributes: These flags only apply to the next entry after them, and may not be repeated. -n, --name - The name to apply to the entry. + The name to apply to the entry. This must be UTF-8 encoded. -s, --symlink Make the next entry into a symlink entry. @@ -295,14 +298,16 @@ Each of these flags creates an entry in the output zip archive. Create a directory entry. A name must be provided beforehand with -n. - -i, --imm - Write an entry containing this data. + -i, --immediate + Write an entry containing the data in the argument, which need not be UTF-8 encoded + but will exit early upon encountering any null bytes. A name must be provided beforehand with -n. -f, --file Write an entry with the contents of this file path. A name may be provided beforehand with -n, otherwise the name will be inferred from relativizing the given path to the working directory. + Note that sockets and pipe inputs are currently not supported and will produce an error. -r, --recursive-dir Write all the recursive contents of this directory path. @@ -316,6 +321,7 @@ Positional entries: If the given path points to a file, then a single file entry will be written. If the given path is a symlink, then a single symlink entry will be written. If the given path refers to a directory, then the recursive contents will be written. + Socket and pipes will produce an error. ", Self::COMMAND_DESCRIPTION, Self::generate_usage_line(), diff --git a/cli/src/compress.rs b/cli/src/compress.rs index bb8c9c198..b3a06c8dd 100644 --- a/cli/src/compress.rs +++ b/cli/src/compress.rs @@ -1,15 +1,16 @@ use std::{ fs, io::{self, Cursor, IsTerminal, Seek, Write}, + mem, path::Path, }; -use color_eyre::eyre::{eyre, Report, WrapErr}; +use color_eyre::eyre::{bail, eyre, Report, WrapErr}; use zip::{ unstable::path_to_string, write::{SimpleFileOptions, ZipWriter}, - CompressionMethod, + CompressionMethod, ZIP64_BYTES_THR, }; use crate::args::*; @@ -61,10 +62,11 @@ fn enter_recursive_dir_entries( )?; writer .add_directory(&base_dirname, options) - .wrap_err_with(|| eyre!("{base_dirname}"))?; + .wrap_err_with(|| eyre!("error adding top-level directory entry {base_dirname}"))?; let mut readdir_stack: Vec<(fs::ReadDir, String)> = vec![( - fs::read_dir(root).wrap_err_with(|| eyre!("{}", root.display()))?, + fs::read_dir(root) + .wrap_err_with(|| eyre!("error reading directory contents for {}", root.display()))?, base_dirname, )]; while let Some((mut readdir, top_component)) = readdir_stack.pop() { @@ -82,29 +84,51 @@ fn enter_recursive_dir_entries( let file_type = dir_entry .file_type() - .wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?; + .wrap_err_with(|| eyre!("failed to read file type for dir entry {dir_entry:?}"))?; if file_type.is_symlink() { let target: String = path_to_string( fs::read_link(dir_entry.path()) - .wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?, + .wrap_err_with(|| eyre!("failed to read symlink from {dir_entry:?}"))?, ) .into(); + if target.len() > ZIP64_BYTES_THR.try_into().unwrap() { + bail!( + "symlink target for {full_path} is over {ZIP64_BYTES_THR} bytes (was: {})", + target.len() + ); + } writeln!( err, "writing recursive symlink entry with name {full_path:?} and target {target:?}" )?; writer .add_symlink(&full_path, &target, options) - .wrap_err_with(|| eyre!("{full_path}->{target}"))?; + .wrap_err_with(|| eyre!("error adding symlink from {full_path}->{target}"))?; } else if file_type.is_file() { writeln!(err, "writing recursive file entry with name {full_path:?}")?; + let mut f = fs::File::open(dir_entry.path()).wrap_err_with(|| { + eyre!("error opening file for {full_path} from dir entry {dir_entry:?}") + })?; + /* Get the length of the file before reading it and set large_file if needed. */ + let input_len: u64 = f + .metadata() + .wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))? + .len(); + let maybe_large_file_options = if input_len > ZIP64_BYTES_THR { + writeln!( + err, + "temporarily ensuring .large_file(true) for current entry" + )?; + options.large_file(true) + } else { + options + }; writer - .start_file(&full_path, options) - .wrap_err_with(|| eyre!("{full_path}"))?; - let mut f = fs::File::open(dir_entry.path()) - .wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?; - io::copy(&mut f, writer) - .wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?; + .start_file(&full_path, maybe_large_file_options) + .wrap_err_with(|| eyre!("error creating file entry for {full_path}"))?; + io::copy(&mut f, writer).wrap_err_with(|| { + eyre!("error copying content for {full_path} from file {f:?}") + })?; } else { assert!(file_type.is_dir()); writeln!( @@ -113,13 +137,14 @@ fn enter_recursive_dir_entries( )?; writer .add_directory(&full_path, options) - .wrap_err_with(|| eyre!("{full_path}"))?; + .wrap_err_with(|| eyre!("failed to create directory entry {full_path}"))?; writeln!( err, "adding subdirectories depth-first for recursive directory entry {entry_basename:?}" )?; - let new_readdir = fs::read_dir(dir_entry.path()) - .wrap_err_with(|| eyre!("{}", dir_entry.path().display()))?; + let new_readdir = fs::read_dir(dir_entry.path()).wrap_err_with(|| { + eyre!("failed to read recursive directory contents from {dir_entry:?}") + })?; readdir_stack.push((new_readdir, entry_basename)); } } @@ -142,7 +167,9 @@ pub fn execute_compress( Some(path) => { writeln!(err, "writing compressed zip to output file path {path:?}")?; OutputHandle::File( - fs::File::create(&path).wrap_err_with(|| eyre!("{}", path.display()))?, + fs::File::create(&path).wrap_err_with(|| { + eyre!("failed to create output file at {}", path.display()) + })?, ) } None => { @@ -213,7 +240,7 @@ pub fn execute_compress( }); writer .add_directory(&dirname, options) - .wrap_err_with(|| eyre!("{dirname}"))?; + .wrap_err_with(|| eyre!("failed to create dir entry {dirname}"))?; } CompressionArg::Symlink => { writeln!(err, "setting symlink flag for next entry")?; @@ -227,6 +254,17 @@ pub fn execute_compress( let name = last_name.take().unwrap_or_else(|| { Compress::exit_arg_invalid("no name provided for immediate data {data:?}") }); + /* It's highly unlikely any OS allows process args of this length, so even though + * we're using rust's env::args_os() and it would be very impressive for an attacker + * to get CLI args to overflow, it seems likely to be inefficient in any case, and + * very unlikely to be useful, so exit with a clear error. */ + if data.len() > ZIP64_BYTES_THR.try_into().unwrap() { + bail!( + "length of immediate data argument is {}; use a file for inputs over {} bytes", + data.len(), + ZIP64_BYTES_THR + ); + }; if symlink_flag { /* This is a symlink entry. */ let target = data.into_string().unwrap_or_else(|target| { @@ -241,7 +279,9 @@ pub fn execute_compress( /* TODO: .add_symlink() should support OsString targets! */ writer .add_symlink(&name, &target, options) - .wrap_err_with(|| eyre!("{name}->{target}"))?; + .wrap_err_with(|| { + eyre!("failed to created symlink entry {name}->{target}") + })?; symlink_flag = false; } else { /* This is a file entry. */ @@ -252,10 +292,13 @@ pub fn execute_compress( let data = data.into_encoded_bytes(); writer .start_file(&name, options) - .wrap_err_with(|| eyre!("{name}"))?; - writer - .write_all(data.as_ref()) - .wrap_err_with(|| eyre!("{name}"))?; + .wrap_err_with(|| eyre!("failed to create file entry {name}"))?; + writer.write_all(data.as_ref()).wrap_err_with(|| { + eyre!( + "failed writing immediate data of length {} to file entry {name}", + data.len() + ) + })?; } } CompressionArg::FilePath(path) => { @@ -264,14 +307,25 @@ pub fn execute_compress( .unwrap_or_else(|| path_to_string(&path).into()); if symlink_flag { /* This is a symlink entry. */ - let target: String = path_to_string( - fs::read_link(&path).wrap_err_with(|| eyre!("{}", path.display()))?, - ) - .into(); + let target: String = + path_to_string(fs::read_link(&path).wrap_err_with(|| { + eyre!("failed to read symlink from path {}", path.display()) + })?) + .into(); + /* Similarly to immediate data arguments, we're simply not going to support + * symlinks over this length, which should be impossible anyway. */ + if target.len() > ZIP64_BYTES_THR.try_into().unwrap() { + bail!( + "symlink target for {name} is over {ZIP64_BYTES_THR} bytes (was: {})", + target.len() + ); + } writeln!(err, "writing symlink entry from path {path:?} with name {name:?} and target {target:?}")?; writer .add_symlink(&name, &target, options) - .wrap_err_with(|| eyre!("{name}->{target}"))?; + .wrap_err_with(|| { + eyre!("failed to create symlink entry for {name}->{target}") + })?; symlink_flag = false; } else { /* This is a file entry. */ @@ -279,12 +333,30 @@ pub fn execute_compress( err, "writing file entry from path {path:?} with name {name:?}" )?; + let mut f = fs::File::open(&path).wrap_err_with(|| { + eyre!("error opening file for {name} at {}", path.display()) + })?; + /* Get the length of the file before reading it and set large_file if needed. */ + let input_len: u64 = f + .metadata() + .wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))? + .len(); + writeln!(err, "entry is {input_len} bytes long")?; + let maybe_large_file_options = if input_len > ZIP64_BYTES_THR { + writeln!( + err, + "temporarily ensuring .large_file(true) for current entry" + )?; + options.large_file(true) + } else { + options + }; writer - .start_file(&name, options) - .wrap_err_with(|| eyre!("{name}"))?; - let mut f = - fs::File::open(&path).wrap_err_with(|| eyre!("{}", path.display()))?; - io::copy(&mut f, &mut writer).wrap_err_with(|| eyre!("{}", path.display()))?; + .start_file(&name, maybe_large_file_options) + .wrap_err_with(|| eyre!("error creating file entry for {name}"))?; + io::copy(&mut f, &mut writer).wrap_err_with(|| { + eyre!("error copying content for {name} from file {f:?}") + })?; } } CompressionArg::RecursiveDirPath(r) => { @@ -296,7 +368,7 @@ pub fn execute_compress( "writing recursive dir entries for path {r:?} with name {last_name:?}" )?; enter_recursive_dir_entries(err, last_name.take(), &r, &mut writer, options) - .wrap_err_with(|| eyre!("{}", r.display()))?; + .wrap_err_with(|| eyre!("failed to read recursive dir {}", r.display()))?; } } } @@ -310,34 +382,62 @@ pub fn execute_compress( } for pos_arg in positional_paths.into_iter() { let file_type = fs::symlink_metadata(&pos_arg) - .wrap_err_with(|| eyre!("{}", pos_arg.display()))? + .wrap_err_with(|| eyre!("failed to read metadata from path {}", pos_arg.display()))? .file_type(); if file_type.is_symlink() { - let target = - fs::read_link(&pos_arg).wrap_err_with(|| eyre!("{}", pos_arg.display()))?; + let target = fs::read_link(&pos_arg).wrap_err_with(|| { + eyre!("failed to read symlink content from {}", pos_arg.display()) + })?; writeln!( err, "writing positional symlink entry with path {pos_arg:?} and target {target:?}" )?; writer .add_symlink_from_path(&pos_arg, &target, options) - .wrap_err_with(|| eyre!("{}->{}", pos_arg.display(), target.display()))?; + .wrap_err_with(|| { + eyre!( + "failed to create symlink entry for {}->{}", + pos_arg.display(), + target.display() + ) + })?; } else if file_type.is_file() { writeln!(err, "writing positional file entry with path {pos_arg:?}")?; + let mut f = fs::File::open(&pos_arg) + .wrap_err_with(|| eyre!("failed to open file at {}", pos_arg.display()))?; + /* Get the length of the file before reading it and set large_file if needed. */ + let input_len: u64 = f + .metadata() + .wrap_err_with(|| eyre!("error reading file metadata for {f:?}"))? + .len(); + let maybe_large_file_options = if input_len > ZIP64_BYTES_THR { + writeln!( + err, + "temporarily ensuring .large_file(true) for current entry" + )?; + options.large_file(true) + } else { + options + }; writer - .start_file_from_path(&pos_arg, options) - .wrap_err_with(|| eyre!("{}", pos_arg.display()))?; - let mut f = - fs::File::open(&pos_arg).wrap_err_with(|| eyre!("{}", pos_arg.display()))?; - io::copy(&mut f, &mut writer).wrap_err_with(|| eyre!("{}", pos_arg.display()))?; + .start_file_from_path(&pos_arg, maybe_large_file_options) + .wrap_err_with(|| eyre!("failed to create file entry {}", pos_arg.display()))?; + io::copy(&mut f, &mut writer) + .wrap_err_with(|| eyre!("failed to copy file contents from {f:?}"))?; } else { assert!(file_type.is_dir()); writeln!( err, "writing positional recursive dir entry for {pos_arg:?}" )?; - enter_recursive_dir_entries(err, None, &pos_arg, &mut writer, options) - .wrap_err_with(|| eyre!("{}", pos_arg.display()))?; + enter_recursive_dir_entries(err, None, &pos_arg, &mut writer, options).wrap_err_with( + || { + eyre!( + "failed to insert recursive directory entries from {}", + pos_arg.display() + ) + }, + )?; } } @@ -345,12 +445,21 @@ pub fn execute_compress( .finish() .wrap_err("failed to write zip to output handle")?; match handle { - OutputHandle::File(_) => (), + OutputHandle::File(f) => { + let archive_len: u64 = f + .metadata() + .wrap_err_with(|| eyre!("failed reading metadata from file {f:?}"))? + .len(); + writeln!(err, "file archive {f:?} was {archive_len} bytes")?; + mem::drop(f); /* Superfluous explicit drop. */ + } OutputHandle::InMem(mut cursor) => { + let archive_len: u64 = cursor.position(); + writeln!(err, "in-memory archive was {archive_len} bytes")?; cursor.rewind().wrap_err("failed to rewind cursor")?; let mut stdout = io::stdout().lock(); io::copy(&mut cursor, &mut stdout) - .wrap_err("failed to copy contents of cursor to stdout")?; + .wrap_err("failed to copy {archive_len} byte archive to stdout")?; } } diff --git a/src/write.rs b/src/write.rs index 40d98937e..8f0a9afcc 100644 --- a/src/write.rs +++ b/src/write.rs @@ -253,6 +253,7 @@ impl<'a> arbitrary::Arbitrary<'a> for EncryptWith<'a> { } /// Metadata for a file to be written +/* TODO: add accessors for this data as well so options can be introspected! */ #[derive(Clone, Debug, Copy, Eq, PartialEq)] pub struct FileOptions<'k, T: FileOptionExtension> { pub(crate) compression_method: CompressionMethod, @@ -1396,6 +1397,7 @@ impl ZipWriter { /// implementations may materialize a symlink as a regular file, possibly with the /// content incorrectly set to the symlink target. For maximum portability, consider /// storing a regular file instead. + /* TODO: support OsStr instead of just str, for non-unicode paths. */ pub fn add_symlink( &mut self, name: N,