diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index e781cc14ba4..4b417f527ee 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -16,7 +16,7 @@ use std::os::unix::net::UnixListener; use std::path::{Path, PathBuf, StripPrefixError}; use std::{fmt, io}; #[cfg(all(unix, not(target_os = "android")))] -use uucore::fsxattr::{copy_xattrs, copy_xattrs_skip_selinux}; +use uucore::fsxattr::{copy_xattrs_fd, copy_xattrs_skip_selinux}; use uucore::translate; use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, value_parser}; @@ -1715,6 +1715,8 @@ pub(crate) fn set_selinux_context(path: &Path, context: Option<&String>) -> Copy /// or if xattr copying fails. #[cfg(all(unix, not(target_os = "android")))] fn copy_extended_attrs(source: &Path, dest: &Path, skip_selinux: bool) -> CopyResult<()> { + use std::fs::File; + use uucore::fsxattr::copy_xattrs; let metadata = fs::symlink_metadata(dest)?; // Check if the destination file is currently read-only for the user. @@ -1734,6 +1736,13 @@ fn copy_extended_attrs(source: &Path, dest: &Path, skip_selinux: bool) -> CopyRe // When -Z is used, skip copying security.selinux xattr so that // the default context can be set instead of preserving from source copy_xattrs_skip_selinux(source, dest) + } else if metadata.is_file() { + // Use file descriptor-based operations for regular files to avoid TOCTOU races. + // Directories cannot be opened with write mode for xattr operations + // Symlinks (especially dangling ones) cannot be opened via File::open + let source_file = File::open(source)?; + let dest_file = OpenOptions::new().write(true).open(dest)?; + copy_xattrs_fd(&source_file, &dest_file) } else { copy_xattrs(source, dest) }; @@ -2591,7 +2600,45 @@ fn copy_file( fs::set_permissions(dest, dest_permissions).ok(); } - let copy_attributes_result = if options.dereference(source_in_command_line) { + let copy_attributes_result = if source_is_stream && options.copy_contents { + // When copying contents from a stream (like a FIFO with --copy-contents), + // we can't re-access the source as it may block. Use the metadata we + // already have to preserve ownership and permissions. + #[cfg(unix)] + { + if let Preserve::Yes { .. } = options.attributes.ownership { + use std::os::unix::prelude::MetadataExt; + use uucore::perms::{Verbosity, VerbosityLevel, wrap_chown}; + + let dest_uid = source_metadata.uid(); + let dest_gid = source_metadata.gid(); + if let Ok(dest_meta) = dest.symlink_metadata() { + let try_chown = |uid| { + wrap_chown( + dest, + &dest_meta, + uid, + Some(dest_gid), + false, + Verbosity { + groups_only: false, + level: VerbosityLevel::Silent, + }, + ) + }; + // gnu compatibility: cp doesn't report an error if it fails to set the + // ownership, and will fall back to changing only the gid if possible. + if try_chown(Some(dest_uid)).is_err() { + let _ = try_chown(None); + } + } + } + if let Preserve::Yes { .. } = options.attributes.mode { + fs::set_permissions(dest, source_metadata.permissions()).ok(); + } + } + Ok(()) + } else if options.dereference(source_in_command_line) { // Try to canonicalize, but if it fails (e.g., due to inaccessible parent directories), // fall back to the original source path let src_for_attrs = canonicalize(source, MissingHandling::Normal, ResolveMode::Physical) @@ -2605,12 +2652,6 @@ fn copy_file( false, options.set_selinux_context, ) - } else if source_is_stream && !source.exists() { - // Some stream files may not exist after we have copied it, - // like anonymous pipes. Thus, we can't really copy its - // attributes. However, this is already handled in the stream - // copy function (see `copy_stream` under platform/linux.rs). - Ok(()) } else { copy_attributes( source, diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 427593ae25d..68d06bc8f76 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -6,19 +6,18 @@ use libc::{SEEK_DATA, SEEK_HOLE}; use std::fs::{File, OpenOptions}; -use std::io::Read; +use std::io::{self, Read}; use std::os::unix::fs::FileExt; use std::os::unix::fs::MetadataExt; use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; use std::os::unix::io::AsRawFd; use std::path::Path; use uucore::buf_copy; -use uucore::mode::get_umask; +use uucore::fs::copy_file_with_secure_permissions; use uucore::translate; use crate::{ CopyDebug, CopyResult, CpError, OffloadReflinkDebug, ReflinkMode, SparseDebug, SparseMode, - is_stream, }; /// The fallback behavior for [`clone`] on failed system call. @@ -54,21 +53,41 @@ enum CopyMethod { /// /// `fallback` controls what to do if the system call fails. #[cfg(any(target_os = "linux", target_os = "android"))] -fn clone

(source: P, dest: P, fallback: CloneFallback) -> std::io::Result<()> +fn clone

(source: P, dest: P, fallback: CloneFallback) -> io::Result<()> where P: AsRef, { let src_file = File::open(&source)?; - let dst_file = File::create(&dest)?; + // Create destination with restrictive permissions initially (to prevent race conditions) + // Mode 0o600 means read/write for owner only + let dst_file = OpenOptions::new() + .create(true) + .write(true) + .truncate(false) + .mode(0o600) + .open(&dest)?; let src_fd = src_file.as_raw_fd(); let dst_fd = dst_file.as_raw_fd(); let result = unsafe { libc::ioctl(dst_fd, libc::FICLONE, src_fd) }; if result == 0 { return Ok(()); } + let err = io::Error::last_os_error(); match fallback { - CloneFallback::Error => Err(std::io::Error::last_os_error()), - CloneFallback::FSCopy => std::fs::copy(source, dest).map(|_| ()), + CloneFallback::Error => { + // FICLONE fails with EINVAL if the destination is not empty. + // Truncate and retry once before giving up. + if err.raw_os_error() == Some(libc::EINVAL) { + dst_file.set_len(0)?; + let retry = unsafe { libc::ioctl(dst_fd, libc::FICLONE, src_fd) }; + if retry == 0 { + return Ok(()); + } + return Err(io::Error::last_os_error()); + } + Err(err) + } + CloneFallback::FSCopy => copy_file_with_secure_permissions(source, dest).map(|_| ()), CloneFallback::SparseCopy => sparse_copy(source, dest), CloneFallback::SparseCopyWithoutHole => sparse_copy_without_hole(source, dest), } @@ -78,7 +97,7 @@ where /// This function returns a tuple of (bool, u64, u64) signifying a tuple of (whether a file has /// data, its size, no of blocks it has allocated in disk) #[cfg(any(target_os = "linux", target_os = "android"))] -fn check_for_data(source: &Path) -> Result<(bool, u64, u64), std::io::Error> { +fn check_for_data(source: &Path) -> Result<(bool, u64, u64), io::Error> { let mut src_file = File::open(source)?; let metadata = src_file.metadata()?; @@ -98,14 +117,14 @@ fn check_for_data(source: &Path) -> Result<(bool, u64, u64), std::io::Error> { match result { -1 => Ok((false, size, blocks)), // No data found or end of file _ if result >= 0 => Ok((true, size, blocks)), // Data found - _ => Err(std::io::Error::last_os_error()), + _ => Err(io::Error::last_os_error()), } } #[cfg(any(target_os = "linux", target_os = "android"))] /// Checks whether a file is sparse i.e. it contains holes, uses the crude heuristic blocks < size / 512 /// Reference:`` -fn check_sparse_detection(source: &Path) -> Result { +fn check_sparse_detection(source: &Path) -> Result { let src_file = File::open(source)?; let metadata = src_file.metadata()?; let size = metadata.size(); @@ -120,17 +139,24 @@ fn check_sparse_detection(source: &Path) -> Result { /// Optimized [`sparse_copy`] doesn't create holes for large sequences of zeros in non `sparse_files` /// Used when `--sparse=auto` #[cfg(any(target_os = "linux", target_os = "android"))] -fn sparse_copy_without_hole

(source: P, dest: P) -> std::io::Result<()> +fn sparse_copy_without_hole

(source: P, dest: P) -> io::Result<()> where P: AsRef, { let src_file = File::open(source)?; - let dst_file = File::create(dest)?; + // Create destination with restrictive permissions initially (to prevent race conditions) + // Mode 0o600 means read/write for owner only + let dst_file = OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .mode(0o600) + .open(dest)?; let dst_fd = dst_file.as_raw_fd(); let size = src_file.metadata()?.size(); if unsafe { libc::ftruncate(dst_fd, size.try_into().unwrap()) } < 0 { - return Err(std::io::Error::last_os_error()); + return Err(io::Error::last_os_error()); } let src_fd = src_file.as_raw_fd(); let mut current_offset: isize = 0; @@ -152,7 +178,7 @@ where break; } if result <= -2 || hole <= -2 { - return Err(std::io::Error::last_os_error()); + return Err(io::Error::last_os_error()); } let len: isize = hole - current_offset; // Read and write data in chunks of `step` while reusing the same buffer @@ -170,17 +196,24 @@ where /// Perform a sparse copy from one file to another. /// Creates a holes for large sequences of zeros in `non_sparse_files`, used for `--sparse=always` #[cfg(any(target_os = "linux", target_os = "android"))] -fn sparse_copy

(source: P, dest: P) -> std::io::Result<()> +fn sparse_copy

(source: P, dest: P) -> io::Result<()> where P: AsRef, { let mut src_file = File::open(source)?; - let dst_file = File::create(dest)?; + // Create destination with restrictive permissions initially (to prevent race conditions) + // Mode 0o600 means read/write for owner only + let dst_file = OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .mode(0o600) + .open(dest)?; let dst_fd = dst_file.as_raw_fd(); let size: usize = src_file.metadata()?.size().try_into().unwrap(); if unsafe { libc::ftruncate(dst_fd, size.try_into().unwrap()) } < 0 { - return Err(std::io::Error::last_os_error()); + return Err(io::Error::last_os_error()); } let blksize = dst_file.metadata()?.blksize(); @@ -214,7 +247,7 @@ fn check_dest_is_fifo(dest: &Path) -> bool { } /// Copy the contents of a stream from `source` to `dest`. -fn copy_stream

(source: P, dest: P) -> std::io::Result +fn copy_stream

(source: P, dest: P) -> io::Result where P: AsRef, { @@ -237,21 +270,18 @@ where // TODO Update the code below to respect the case where // `--preserve=ownership` is not true. let mut src_file = File::open(&source)?; - let mode = 0o622 & !get_umask(); + // Create with restrictive permissions initially to prevent race conditions + // Mode 0o600 means read/write for owner only + // Use truncate(true) to ensure we overwrite existing content let mut dst_file = OpenOptions::new() .create(true) .write(true) - .mode(mode) + .truncate(true) + .mode(0o600) .open(&dest)?; - let dest_is_stream = is_stream(&dst_file.metadata()?); - if !dest_is_stream { - // `copy_stream` doesn't clear the dest file, if dest is not a stream, we should clear it manually. - dst_file.set_len(0)?; - } - let num_bytes_copied = buf_copy::copy_stream(&mut src_file, &mut dst_file) - .map_err(|e| std::io::Error::other(format!("{e}")))?; + .map_err(|e| io::Error::other(format!("{e}")))?; Ok(num_bytes_copied) } @@ -287,7 +317,9 @@ pub(crate) fn copy_on_write( } match copy_method { - CopyMethod::FSCopy => std::fs::copy(source, dest).map(|_| ()), + CopyMethod::FSCopy => { + copy_file_with_secure_permissions(source, dest).map(|_| ()) + } _ => sparse_copy(source, dest), } } @@ -303,7 +335,7 @@ pub(crate) fn copy_on_write( if let Ok(debug) = result { copy_debug = debug; } - std::fs::copy(source, dest).map(|_| ()) + copy_file_with_secure_permissions(source, dest).map(|_| ()) } } (ReflinkMode::Never, SparseMode::Auto) => { @@ -322,7 +354,7 @@ pub(crate) fn copy_on_write( match copy_method { CopyMethod::SparseCopyWithoutHole => sparse_copy_without_hole(source, dest), - _ => std::fs::copy(source, dest).map(|_| ()), + _ => copy_file_with_secure_permissions(source, dest).map(|_| ()), } } } @@ -401,7 +433,7 @@ pub(crate) fn copy_on_write( fn handle_reflink_auto_sparse_always( source: &Path, dest: &Path, -) -> Result<(CopyDebug, CopyMethod), std::io::Error> { +) -> Result<(CopyDebug, CopyMethod), io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::Unsupported, @@ -438,7 +470,7 @@ fn handle_reflink_auto_sparse_always( /// Handles debug results when flags are "--reflink=auto" and "--sparse=auto" and specifies what /// type of copy should be used -fn handle_reflink_never_sparse_never(source: &Path) -> Result { +fn handle_reflink_never_sparse_never(source: &Path) -> Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, @@ -459,7 +491,7 @@ fn handle_reflink_never_sparse_never(source: &Path) -> Result Result { +fn handle_reflink_auto_sparse_never(source: &Path) -> Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, @@ -484,7 +516,7 @@ fn handle_reflink_auto_sparse_never(source: &Path) -> Result Result<(CopyDebug, CopyMethod), std::io::Error> { +) -> Result<(CopyDebug, CopyMethod), io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::Unsupported, @@ -527,7 +559,7 @@ fn handle_reflink_auto_sparse_auto( fn handle_reflink_never_sparse_auto( source: &Path, dest: &Path, -) -> Result<(CopyDebug, CopyMethod), std::io::Error> { +) -> Result<(CopyDebug, CopyMethod), io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, @@ -563,7 +595,7 @@ fn handle_reflink_never_sparse_auto( fn handle_reflink_never_sparse_always( source: &Path, dest: &Path, -) -> Result<(CopyDebug, CopyMethod), std::io::Error> { +) -> Result<(CopyDebug, CopyMethod), io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, diff --git a/src/uu/cp/src/platform/macos.rs b/src/uu/cp/src/platform/macos.rs index a545c552ba4..abb24524560 100644 --- a/src/uu/cp/src/platform/macos.rs +++ b/src/uu/cp/src/platform/macos.rs @@ -5,19 +5,18 @@ // spell-checker:ignore reflink use std::ffi::CString; use std::fs::{self, File, OpenOptions}; +use std::io; use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::OpenOptionsExt; use std::path::Path; use uucore::buf_copy; use uucore::display::Quotable; +use uucore::fs::copy_file_with_secure_permissions; use uucore::translate; -use uucore::mode::get_umask; - use crate::{ CopyDebug, CopyResult, CpError, OffloadReflinkDebug, ReflinkMode, SparseDebug, SparseMode, - is_stream, }; /// Copies `source` to `dest` using copy-on-write if possible. @@ -63,7 +62,7 @@ pub(crate) fn copy_on_write( flags: u32, ) -> libc::c_int = std::mem::transmute(raw_pfn); error = pfn(src.as_ptr(), dst.as_ptr(), 0); - if std::io::Error::last_os_error().kind() == std::io::ErrorKind::AlreadyExists + if io::Error::last_os_error().kind() == io::ErrorKind::AlreadyExists // Only remove the `dest` if the `source` and `dest` are not the same && source != dest { @@ -92,24 +91,21 @@ pub(crate) fn copy_on_write( copy_debug.reflink = OffloadReflinkDebug::Yes; if source_is_stream { let mut src_file = File::open(source)?; - let mode = 0o622 & !get_umask(); + // Create with restrictive permissions initially to prevent race conditions + // Mode 0o600 means read/write for owner only let mut dst_file = OpenOptions::new() .create(true) .write(true) - .mode(mode) + .truncate(true) + .mode(0o600) .open(dest)?; - let dest_is_stream = is_stream(&dst_file.metadata()?); - if !dest_is_stream { - // `copy_stream` doesn't clear the dest file, if dest is not a stream, we should clear it manually. - dst_file.set_len(0)?; - } - buf_copy::copy_stream(&mut src_file, &mut dst_file) - .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other)) + .map_err(|_| io::Error::from(io::ErrorKind::Other)) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; } else { - fs::copy(source, dest).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + copy_file_with_secure_permissions(source, dest) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; } } diff --git a/src/uu/cp/src/platform/other_unix.rs b/src/uu/cp/src/platform/other_unix.rs index 2db85c56af1..1d60e60ebfb 100644 --- a/src/uu/cp/src/platform/other_unix.rs +++ b/src/uu/cp/src/platform/other_unix.rs @@ -3,17 +3,17 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. // spell-checker:ignore reflink -use std::fs::{self, File, OpenOptions}; +use std::fs::{File, OpenOptions}; +use std::io; use std::os::unix::fs::OpenOptionsExt; use std::path::Path; use uucore::buf_copy; -use uucore::mode::get_umask; +use uucore::fs::copy_file_with_secure_permissions; use uucore::translate; use crate::{ CopyDebug, CopyResult, CpError, OffloadReflinkDebug, ReflinkMode, SparseDebug, SparseMode, - is_stream, }; /// Copies `source` to `dest` for systems without copy-on-write @@ -43,27 +43,24 @@ pub(crate) fn copy_on_write( if source_is_stream { let mut src_file = File::open(source)?; - let mode = 0o622 & !get_umask(); + // Create with restrictive permissions initially to prevent race conditions + // Mode 0o600 means read/write for owner only let mut dst_file = OpenOptions::new() .create(true) .write(true) - .mode(mode) + .truncate(true) + .mode(0o600) .open(dest)?; - let dest_is_stream = is_stream(&dst_file.metadata()?); - if !dest_is_stream { - // `copy_stream` doesn't clear the dest file, if dest is not a stream, we should clear it manually. - dst_file.set_len(0)?; - } - buf_copy::copy_stream(&mut src_file, &mut dst_file) - .map_err(|_| std::io::Error::from(std::io::ErrorKind::Other)) + .map_err(|_| io::Error::from(io::ErrorKind::Other)) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; return Ok(copy_debug); } - fs::copy(source, dest).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + copy_file_with_secure_permissions(source, dest) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; Ok(copy_debug) } diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 2f69801e15a..aa5005a4697 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) sourcepath targetpath nushell canonicalized unwriteable +// spell-checker:ignore (ToDO) sourcepath targetpath nushell canonicalized unwriteable lgetxattr lsetxattr mod error; #[cfg(unix)] @@ -901,9 +901,15 @@ fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> { fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { let path_symlink_points_to = fs::read_link(from)?; unix::fs::symlink(path_symlink_points_to, to)?; + // Use path-based xattr operations for symlinks because File::open() + // follows the symlink to its target (which may be dangling). + // xattr::list/get/set operate on the symlink itself (lgetxattr/lsetxattr). #[cfg(not(any(target_os = "macos", target_os = "redox")))] { - let _ = copy_xattrs_if_supported(from, to); + match fsxattr::copy_xattrs(from, to) { + Err(e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => {} + Err(_) | Ok(()) => {} + } } fs::remove_file(from) } @@ -967,8 +973,15 @@ fn rename_dir_fallback( (_, _) => None, }; + // Retrieve xattrs before copying (directories use path-based operations + // since they cannot be opened in write mode for xattr operations) #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] - let xattrs = fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| FxHashMap::default()); + let xattrs = { + use std::fs::File; + File::open(from) + .and_then(|f| fsxattr::retrieve_xattrs_fd(&f)) + .unwrap_or_else(|_| FxHashMap::default()) + }; // Use directory copying (with or without hardlink support) let result = copy_dir_contents( @@ -983,8 +996,14 @@ fn rename_dir_fallback( display_manager, ); + // Apply xattrs after directory contents are copied, ignoring ENOTSUP errors + // (filesystem doesn't support xattrs, which is acceptable for cross-device moves) #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] - fsxattr::apply_xattrs(to, xattrs)?; + if let Err(e) = fsxattr::apply_xattrs(to, xattrs) { + if e.raw_os_error() != Some(libc::EOPNOTSUPP) { + return Err(e); + } + } result?; @@ -1082,7 +1101,16 @@ fn copy_dir_contents_recursive( rename_symlink_fallback(&from_path, &to_path)?; } - print_verbose(&from_path, &to_path); + // Print verbose message for symlink + if verbose { + let message = translate!("mv-verbose-renamed", "from" => from_path.quote(), "to" => to_path.quote()); + match display_manager { + Some(pb) => pb.suspend(|| { + println!("{message}"); + }), + None => println!("{message}"), + } + } } else if from_path.is_dir() { // Recursively copy subdirectory (only real directories, not symlinks) fs::create_dir_all(&to_path)?; @@ -1215,12 +1243,19 @@ fn rename_file_fallback( Ok(()) } -/// Copy xattrs from source to destination, ignoring ENOTSUP/EOPNOTSUPP errors. +/// Copy xattrs from source to destination using file descriptors, ignoring ENOTSUP/EOPNOTSUPP errors. +/// This version avoids TOCTOU races by operating on file descriptors rather than paths. /// These errors indicate the filesystem doesn't support extended attributes, /// which is acceptable when moving files across filesystems. #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))] fn copy_xattrs_if_supported(from: &Path, to: &Path) -> io::Result<()> { - match fsxattr::copy_xattrs(from, to) { + use std::fs::{File, OpenOptions}; + + // Open both files to pin their inodes, avoiding TOCTOU races during xattr operations + let source_file = File::open(from)?; + let dest_file = OpenOptions::new().write(true).open(to)?; + + match fsxattr::copy_xattrs_fd(&source_file, &dest_file) { Ok(()) => Ok(()), Err(e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => Ok(()), Err(e) => Err(e), diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 6ef229d175c..1b62f3d52ae 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -900,6 +900,23 @@ pub fn makedev(maj: libc::c_uint, min: libc::c_uint) -> libc::dev_t { (min & 0xff) | ((maj & 0xfff) << 8) | ((min & !0xff) << 12) | ((maj & !0xfff) << 32) } +/// Copy file contents with restrictive permissions initially to prevent race conditions. +/// Creates the destination file with mode 0o600 (read/write for owner only), +/// copies the content, and lets the caller set the final permissions afterward. +#[cfg(unix)] +pub fn copy_file_with_secure_permissions>(source: P, dest: P) -> IOResult { + use std::os::unix::fs::OpenOptionsExt; + + let mut src_file = fs::File::open(&source)?; + let mut dst_file = fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .mode(0o600) + .open(&dest)?; + std::io::copy(&mut src_file, &mut dst_file) +} + #[cfg(test)] mod tests { // Note this useful idiom: importing names from outer (for mod tests) scope. @@ -1196,4 +1213,35 @@ mod tests { std::thread::spawn(move || assert!(fs::write(&path2, b"foo").is_ok())); assert_eq!(fs::read(&path).unwrap(), b"foo"); } + + #[test] + #[cfg(unix)] + fn test_copy_file_with_secure_permissions() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempdir().unwrap(); + let src = dir.path().join("src.txt"); + let dst = dir.path().join("dst.txt"); + + fs::write(&src, b"hello world").unwrap(); + + let bytes = copy_file_with_secure_permissions(&src, &dst).unwrap(); + assert_eq!(bytes, 11); + assert_eq!(fs::read(&dst).unwrap(), b"hello world"); + + let mode = fs::metadata(&dst).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + } + + #[test] + #[cfg(unix)] + fn test_copy_file_with_secure_permissions_nonexistent_source() { + let dir = tempdir().unwrap(); + let src = dir.path().join("nonexistent.txt"); + let dst = dir.path().join("dst.txt"); + + let result = copy_file_with_secure_permissions(&src, &dst); + assert!(result.is_err()); + assert_eq!(result.unwrap_err().kind(), ErrorKind::NotFound); + } } diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index d9683264652..e2fc76d9c78 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -9,9 +9,11 @@ use itertools::Itertools; use rustc_hash::FxHashMap; use std::ffi::{OsStr, OsString}; +use std::fs::File; #[cfg(unix)] use std::os::unix::ffi::OsStrExt; use std::path::Path; +use xattr::FileExt; /// Copies extended attributes (xattrs) from one file or directory to another. /// @@ -45,6 +47,29 @@ pub fn copy_xattrs_skip_selinux>(source: P, dest: P) -> std::io:: Ok(()) } +/// Copies extended attributes (xattrs) from one file to another using file descriptors. +/// +/// This version avoids TOCTOU (time-of-check to time-of-use) races by operating +/// on open file descriptors rather than paths, ensuring all operations target +/// the same inodes throughout. +/// +/// # Arguments +/// +/// * `source` - A reference to the source file (open file descriptor). +/// * `dest` - A reference to the destination file (open file descriptor). +/// +/// # Returns +/// +/// A result indicating success or failure. +pub fn copy_xattrs_fd(source: &File, dest: &File) -> std::io::Result<()> { + for attr_name in source.list_xattr()? { + if let Some(value) = source.get_xattr(&attr_name)? { + dest.set_xattr(&attr_name, &value)?; + } + } + Ok(()) +} + /// Retrieves the extended attributes (xattrs) of a given file or directory. /// /// # Arguments @@ -64,6 +89,28 @@ pub fn retrieve_xattrs>(source: P) -> std::io::Result std::io::Result>> { + let mut attrs = FxHashMap::default(); + for attr_name in source.list_xattr()? { + if let Some(value) = source.get_xattr(&attr_name)? { + attrs.insert(attr_name, value); + } + } + Ok(attrs) +} + /// Applies extended attributes (xattrs) to a given file or directory. /// /// # Arguments @@ -84,6 +131,26 @@ pub fn apply_xattrs>( Ok(()) } +/// Applies extended attributes (xattrs) to a given file using a file descriptor. +/// +/// This version avoids TOCTOU races by operating on an open file descriptor +/// rather than a path, ensuring all operations target the same inode. +/// +/// # Arguments +/// +/// * `dest` - A reference to the file (open file descriptor). +/// * `xattrs` - A HashMap containing attribute names and their corresponding values. +/// +/// # Returns +/// +/// A result indicating success or failure. +pub fn apply_xattrs_fd(dest: &File, xattrs: FxHashMap>) -> std::io::Result<()> { + for (attr, value) in xattrs { + dest.set_xattr(&attr, &value)?; + } + Ok(()) +} + /// Checks if a file has an Access Control List (ACL) based on its extended attributes. /// /// # Arguments @@ -299,4 +366,59 @@ mod tests { assert!(has_security_cap_acl(&file_path)); } } + + #[test] + fn test_copy_xattrs_fd() { + use std::fs::OpenOptions; + + let temp_dir = tempdir().unwrap(); + let source_path = temp_dir.path().join("source.txt"); + let dest_path = temp_dir.path().join("dest.txt"); + + File::create(&source_path).unwrap(); + File::create(&dest_path).unwrap(); + + let test_attr = "user.test_fd"; + let test_value = b"test value fd"; + xattr::set(&source_path, test_attr, test_value).unwrap(); + + let source_file = File::open(&source_path).unwrap(); + let dest_file = OpenOptions::new().write(true).open(&dest_path).unwrap(); + + copy_xattrs_fd(&source_file, &dest_file).unwrap(); + + let copied_value = xattr::get(&dest_path, test_attr).unwrap().unwrap(); + assert_eq!(copied_value, test_value); + } + + #[test] + fn test_apply_and_retrieve_xattrs_fd() { + use std::fs::OpenOptions; + + let temp_dir = tempdir().unwrap(); + let file_path = temp_dir.path().join("test_file.txt"); + + File::create(&file_path).unwrap(); + + let mut test_xattrs = FxHashMap::default(); + let test_attr = "user.test_attr_fd"; + let test_value = b"test value fd"; + test_xattrs.insert(OsString::from(test_attr), test_value.to_vec()); + + // Apply using file descriptor + let file = OpenOptions::new().write(true).open(&file_path).unwrap(); + apply_xattrs_fd(&file, test_xattrs).unwrap(); + drop(file); + + // Retrieve using file descriptor + let file = File::open(&file_path).unwrap(); + let retrieved_xattrs = retrieve_xattrs_fd(&file).unwrap(); + assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str())); + assert_eq!( + retrieved_xattrs + .get(OsString::from(test_attr).as_os_str()) + .unwrap(), + test_value + ); + } } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 28af3075eef..32c20357952 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -7877,3 +7877,75 @@ fn test_cp_recursive_non_utf8_source() { assert!(at.plus("dir2").join("a").exists()); } + +#[test] +#[cfg(all( + unix, + not(any(target_os = "android", target_os = "macos", target_os = "openbsd")) +))] +fn test_cp_xattr_implicit_vs_explicit() { + // Test the difference between implicit (-a) and explicit (--preserve=xattr) xattr handling + use std::process::Command; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let source_file = "source.txt"; + at.write(source_file, "test content"); + + // Try to set an xattr on the source file + let xattr_key = "user.test"; + let xattr_value = "test_value"; + + let xattr_set = Command::new("setfattr") + .args([ + "-n", + xattr_key, + "-v", + xattr_value, + &at.plus_as_string(source_file), + ]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false); + + if !xattr_set { + eprintln!("Skipping test: Cannot set xattrs"); + return; + } + + // Test 1: cp -a (implicit xattr preservation - should never fail) + scene + .ucmd() + .args(&["-a", source_file, "dest_implicit.txt"]) + .succeeds() + .no_stderr(); // Should not produce errors for implicit preservation + + assert!(at.file_exists("dest_implicit.txt")); + + // Test 2: cp --preserve=xattr (explicit - may fail if filesystem doesn't support) + // This should succeed on normal filesystems but the behavior differs from -a + // in that explicit --preserve=xattr is "required" while -a is "best effort" + let result = scene + .ucmd() + .args(&["--preserve=xattr", source_file, "dest_explicit.txt"]) + .run(); + + // The file should be created either way + if at.file_exists("dest_explicit.txt") { + // Check if xattrs were actually preserved + let getfattr_check = Command::new("getfattr") + .args(["-n", xattr_key, &at.plus_as_string("dest_explicit.txt")]) + .output() + .unwrap(); + + if getfattr_check.status.success() { + // xattrs were preserved successfully + assert!( + result.succeeded(), + "cp --preserve=xattr should succeed when xattrs can be preserved" + ); + } + // If xattrs weren't preserved, the behavior depends on the filesystem + } +} diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 80448ba1b84..7de9b29fe78 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2882,6 +2882,7 @@ fn test_mv_cross_device_symlink_preserved() { at.write("src_dir/local.txt", "local content"); symlink("/etc", at.plus("src_dir/etc_link")).expect("Failed to create symlink"); + // Verify initial state assert!(at.is_symlink("src_dir/etc_link")); // Force cross-filesystem move using /dev/shm (tmpfs) @@ -2896,6 +2897,7 @@ fn test_mv_cross_device_symlink_preserved() { .succeeds() .no_stderr(); + // Verify source was removed assert!(!at.dir_exists("src_dir")); // Verify the symlink was preserved (not expanded) @@ -3006,3 +3008,57 @@ fn test_mv_cross_device_file_symlink_preserved() { "target content" ); } + +#[test] +#[cfg(all(target_os = "linux", not(target_os = "android")))] +fn test_mv_cross_device_xattr_basic() { + // Basic test that mv handles xattrs correctly during cross-device moves + use std::process::Command; + use tempfile::TempDir; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create a source file + let source_file = "source_xattr.txt"; + at.write(source_file, "test content"); + + // Try to set an xattr + let xattr_key = "user.test"; + let xattr_value = "test_value"; + + let xattr_set = Command::new("setfattr") + .args([ + "-n", + xattr_key, + "-v", + xattr_value, + &at.plus_as_string(source_file), + ]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false); + + if !xattr_set { + eprintln!("Skipping test: Cannot set xattrs"); + return; + } + + // Create target on different filesystem (tmpfs) + let Ok(target_dir) = TempDir::new_in("/dev/shm/") else { + eprintln!("Skipping test: Cannot create directory in /dev/shm"); + return; + }; + + let target_path = target_dir.path().join("moved.txt"); + + // Move should succeed even if xattrs can't be preserved + scene + .ucmd() + .arg(at.plus_as_string(source_file)) + .arg(target_path.to_str().unwrap()) + .succeeds(); + + assert!(!at.file_exists(source_file)); + assert!(target_path.exists()); +}