diff --git a/src/uu/cp/Cargo.toml b/src/uu/cp/Cargo.toml index dff52b4be9c..a184ee887b4 100644 --- a/src/uu/cp/Cargo.toml +++ b/src/uu/cp/Cargo.toml @@ -32,6 +32,7 @@ uucore = { workspace = true, features = [ "parser", "perms", "mode", + "safe-copy", "update-control", ] } walkdir = { workspace = true } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index c48cfda52ed..21adaabd9bb 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_acls, copy_xattrs, copy_xattrs_skip_selinux}; use uucore::translate; use clap::{Arg, ArgAction, ArgMatches, Command, builder::ValueParser, value_parser}; @@ -921,13 +921,15 @@ impl Attributes { xattr: Preserve::No { explicit: false }, }; - // TODO: ownership is required if the user is root, for non-root users it's not required. + // xattr is intentionally NOT in DEFAULT — GNU `cp -p` only preserves + // mode, ownership, and timestamps. Default xattr preservation leaks + // capability / SELinux labels into copies and fails hard on filesystems + // without xattr support. See issue #9704. pub const DEFAULT: Self = Self { #[cfg(unix)] ownership: Preserve::Yes { required: true }, mode: Preserve::Yes { required: true }, timestamps: Preserve::Yes { required: true }, - xattr: Preserve::Yes { required: true }, ..Self::NONE }; @@ -1778,6 +1780,14 @@ pub(crate) fn copy_attributes( attributes.mode }; + // Track whether `chown` to the source's uid succeeded. If it did not + // (typical case: non-root user copying a root-owned setuid file), the + // mode preservation below must strip setuid/setgid so the destination + // does not give the copying user elevated privileges via the copy. + // Matches GNU cp. See issue #9750. + #[cfg(unix)] + let ownership_preserved = std::cell::Cell::new(true); + // Ownership must be changed first to avoid interfering with mode change. #[cfg(unix)] handle_preserve(attributes.ownership, || -> CopyResult<()> { @@ -1810,6 +1820,7 @@ pub(crate) fn copy_attributes( // 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() { + ownership_preserved.set(false); let _ = try_chown(None); } Ok(()) @@ -1822,13 +1833,37 @@ pub(crate) fn copy_attributes( // do nothing, since every symbolic link has the same // permissions. if !dest.is_symlink() { - fs::set_permissions(dest, source_metadata.permissions()) + #[cfg(unix)] + let source_perms = { + use std::os::unix::fs::PermissionsExt; + let mut perms = source_metadata.permissions(); + if !ownership_preserved.get() { + // GNU cp strips setuid (04000) and setgid (02000) when + // ownership could not be preserved. Keep the sticky bit + // (01000) and all rwx bits. + let mode = perms.mode() & !0o6000; + perms.set_mode(mode); + } + perms + }; + #[cfg(not(unix))] + let source_perms = source_metadata.permissions(); + + fs::set_permissions(dest, source_perms) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; // FIXME: Implement this for windows as well #[cfg(feature = "feat_acl")] exacl::getfacl(source, None) .and_then(|acl| exacl::setfacl(&[dest], &acl, None)) .map_err(|err| CpError::Error(err.to_string()))?; + // GNU `cp -p` preserves POSIX ACLs as part of mode. On Linux the + // ACLs are stored as `system.posix_acl_*` xattrs; copy just those + // so we keep ACL parity with GNU without preserving user xattrs + // (which are intentionally excluded from the default -p set per + // issue #9704). Best-effort: ignore failures on filesystems that + // do not support ACL xattrs. + #[cfg(all(unix, not(target_os = "android")))] + copy_acls(source, dest); } Ok(()) @@ -2274,6 +2309,7 @@ fn handle_copy_mode( context, source_metadata, symlinked_files, + source_in_command_line, created_parent_dirs, )?; } @@ -2294,6 +2330,7 @@ fn handle_copy_mode( context, source_metadata, symlinked_files, + source_in_command_line, created_parent_dirs, )?; } @@ -2327,6 +2364,7 @@ fn handle_copy_mode( context, source_metadata, symlinked_files, + source_in_command_line, created_parent_dirs, )?; } @@ -2339,6 +2377,7 @@ fn handle_copy_mode( context, source_metadata, symlinked_files, + source_in_command_line, created_parent_dirs, )?; } @@ -2716,6 +2755,7 @@ fn handle_no_preserve_mode(options: &Options, org_mode: u32) -> u32 { /// Copy the file from `source` to `dest` either using the normal `fs::copy` or a /// copy-on-write scheme if --reflink is specified and the filesystem supports it. +#[allow(clippy::too_many_arguments)] fn copy_helper( source: &Path, dest: &Path, @@ -2723,6 +2763,7 @@ fn copy_helper( context: &str, source_metadata: &Metadata, symlinked_files: &mut HashSet, + #[cfg_attr(not(unix), allow(unused_variables))] source_in_command_line: bool, created_parent_dirs: &mut HashSet, ) -> CopyResult<()> { if options.parents { @@ -2753,6 +2794,14 @@ fn copy_helper( if source_metadata.is_symlink() { copy_link(source, dest, symlinked_files, options)?; } else { + // Use O_NOFOLLOW on the source open iff cp is in no-dereference mode. + // In that case source_metadata was obtained via lstat, so a path swap + // to a symlink between lstat and open must be refused to close the + // TOCTOU window described in issue #10017. In deref mode cp + // intentionally follows symlinks, matching GNU cp's behavior of + // applying O_NOFOLLOW here only with `-P`. + #[cfg(unix)] + let nofollow = !options.dereference(source_in_command_line); let copy_debug = copy_on_write( source, dest, @@ -2761,6 +2810,8 @@ fn copy_helper( context, #[cfg(unix)] is_stream(source_metadata), + #[cfg(unix)] + nofollow, )?; if !options.attributes_only && options.debug { diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 046b925d5fb..328c3f4a112 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -2,17 +2,17 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore ficlone reflink ftruncate pwrite fiemap lseek +// spell-checker:ignore ficlone reflink ftruncate pwrite fiemap lseek nofollow use rustix::fs::{SeekFrom, ftruncate, ioctl_ficlone, seek}; -use std::fs::{File, OpenOptions}; use std::io::Read; use std::os::unix::fs::FileExt; +use std::os::unix::fs::FileTypeExt; use std::os::unix::fs::MetadataExt; -use std::os::unix::fs::{FileTypeExt, OpenOptionsExt}; use std::path::Path; + use uucore::buf_copy; -use uucore::mode::get_umask; +use uucore::safe_copy::{create_dest_restrictive, open_source}; use uucore::translate; use crate::{ @@ -20,6 +20,20 @@ use crate::{ is_stream, }; +// Replacement for `std::fs::copy` that uses the safe-copy primitives but +// only applies `O_NOFOLLOW` to the *source* open. The destination is +// followed if it is a pre-existing symlink, matching GNU cp -d/-P which +// only forbid dereferencing on the source side. +fn fs_copy(source: P, dest: Q, source_nofollow: bool) -> std::io::Result +where + P: AsRef, + Q: AsRef, +{ + let mut src = open_source(source, source_nofollow)?; + let mut dst = create_dest_restrictive(dest, false)?; + std::io::copy(&mut src, &mut dst) +} + /// The fallback behavior for [`clone`] on failed system call. #[derive(Clone, Copy)] enum CloneFallback { @@ -53,18 +67,20 @@ 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, nofollow: bool) -> std::io::Result<()> where P: AsRef, { - let src_file = File::open(&source)?; - let dst_file = File::create(&dest)?; + let src_file = open_source(&source, nofollow)?; + let dst_file = create_dest_restrictive(&dest, false)?; if ioctl_ficlone(dst_file, src_file).is_err() { return match fallback { CloneFallback::Error => Err(std::io::Error::last_os_error()), - CloneFallback::FSCopy => std::fs::copy(source, dest).map(|_| ()), - CloneFallback::SparseCopy => sparse_copy(source, dest), - CloneFallback::SparseCopyWithoutHole => sparse_copy_without_hole(source, dest), + CloneFallback::FSCopy => fs_copy(source, dest, nofollow).map(|_| ()), + CloneFallback::SparseCopy => sparse_copy(source, dest, nofollow), + CloneFallback::SparseCopyWithoutHole => { + sparse_copy_without_hole(source, dest, nofollow) + } }; } Ok(()) @@ -74,8 +90,8 @@ 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> { - let mut src_file = File::open(source)?; +fn check_for_data(source: &Path, nofollow: bool) -> Result<(bool, u64, u64), std::io::Error> { + let mut src_file = open_source(source, nofollow)?; let metadata = src_file.metadata()?; let size = metadata.size(); @@ -94,8 +110,8 @@ fn check_for_data(source: &Path) -> Result<(bool, u64, u64), std::io::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 { - let src_file = File::open(source)?; +fn check_sparse_detection(source: &Path, nofollow: bool) -> Result { + let src_file = open_source(source, nofollow)?; let metadata = src_file.metadata()?; let size = metadata.size(); let blocks = metadata.blocks(); @@ -109,12 +125,12 @@ 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, nofollow: bool) -> std::io::Result<()> where P: AsRef, { - let src_file = File::open(source)?; - let dst_file = File::create(dest)?; + let src_file = open_source(source, nofollow)?; + let dst_file = create_dest_restrictive(dest, false)?; let size = src_file.metadata()?.size(); ftruncate(&dst_file, size)?; @@ -144,12 +160,12 @@ 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, nofollow: bool) -> std::io::Result<()> where P: AsRef, { - let mut src_file = File::open(source)?; - let dst_file = File::create(dest)?; + let mut src_file = open_source(source, nofollow)?; + let dst_file = create_dest_restrictive(dest, false)?; let size: usize = src_file.metadata()?.size().try_into().unwrap(); ftruncate(&dst_file, size.try_into().unwrap())?; @@ -185,7 +201,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, nofollow: bool) -> std::io::Result<()> where P: AsRef, { @@ -207,13 +223,12 @@ 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(); - let mut dst_file = OpenOptions::new() - .create(true) - .write(true) - .mode(mode) - .open(&dest)?; + let mut src_file = open_source(&source, nofollow)?; + // Use the same restrictive initial mode as the regular file path so that + // the dest does not momentarily sit with broader perms. The `0o622 & + // !umask` form previously used here could still allow group/other write + // under a permissive umask. See #10011. + let mut dst_file = create_dest_restrictive(&dest, false)?; let dest_is_stream = is_stream(&dst_file.metadata()?); if !dest_is_stream { @@ -235,6 +250,7 @@ pub(crate) fn copy_on_write( sparse_mode: SparseMode, context: &str, source_is_stream: bool, + nofollow: bool, ) -> CopyResult { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -248,18 +264,18 @@ pub(crate) fn copy_on_write( copy_debug.reflink = OffloadReflinkDebug::No; if source_is_stream { copy_debug.offload = OffloadReflinkDebug::Avoided; - copy_stream(source, dest).map(|_| ()) + copy_stream(source, dest, nofollow).map(|_| ()) } else { let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_always(source, dest); + let result = handle_reflink_never_sparse_always(source, dest, nofollow); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } match copy_method { - CopyMethod::FSCopy => std::fs::copy(source, dest).map(|_| ()), - _ => sparse_copy(source, dest), + CopyMethod::FSCopy => fs_copy(source, dest, nofollow).map(|_| ()), + _ => sparse_copy(source, dest, nofollow), } } } @@ -268,13 +284,13 @@ pub(crate) fn copy_on_write( if source_is_stream { copy_debug.offload = OffloadReflinkDebug::Avoided; - copy_stream(source, dest).map(|_| ()) + copy_stream(source, dest, nofollow).map(|_| ()) } else { - let result = handle_reflink_never_sparse_never(source); + let result = handle_reflink_never_sparse_never(source, nofollow); if let Ok(debug) = result { copy_debug = debug; } - std::fs::copy(source, dest).map(|_| ()) + fs_copy(source, dest, nofollow).map(|_| ()) } } (ReflinkMode::Never, SparseMode::Auto) => { @@ -282,18 +298,20 @@ pub(crate) fn copy_on_write( if source_is_stream { copy_debug.offload = OffloadReflinkDebug::Avoided; - copy_stream(source, dest).map(|_| ()) + copy_stream(source, dest, nofollow).map(|_| ()) } else { let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_auto(source, dest); + let result = handle_reflink_never_sparse_auto(source, dest, nofollow); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } match copy_method { - CopyMethod::SparseCopyWithoutHole => sparse_copy_without_hole(source, dest), - _ => std::fs::copy(source, dest).map(|_| ()), + CopyMethod::SparseCopyWithoutHole => { + sparse_copy_without_hole(source, dest, nofollow) + } + _ => fs_copy(source, dest, nofollow).map(|_| ()), } } } @@ -302,18 +320,18 @@ pub(crate) fn copy_on_write( // SparseMode::Always if source_is_stream { copy_debug.offload = OffloadReflinkDebug::Avoided; - copy_stream(source, dest).map(|_| ()) + copy_stream(source, dest, nofollow).map(|_| ()) } else { let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_always(source, dest); + let result = handle_reflink_auto_sparse_always(source, dest, nofollow); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; } match copy_method { - CopyMethod::FSCopy => clone(source, dest, CloneFallback::FSCopy), - _ => clone(source, dest, CloneFallback::SparseCopy), + CopyMethod::FSCopy => clone(source, dest, CloneFallback::FSCopy, nofollow), + _ => clone(source, dest, CloneFallback::SparseCopy, nofollow), } } } @@ -322,23 +340,23 @@ pub(crate) fn copy_on_write( copy_debug.reflink = OffloadReflinkDebug::No; if source_is_stream { copy_debug.offload = OffloadReflinkDebug::Avoided; - copy_stream(source, dest).map(|_| ()) + copy_stream(source, dest, nofollow).map(|_| ()) } else { - let result = handle_reflink_auto_sparse_never(source); + let result = handle_reflink_auto_sparse_never(source, nofollow); if let Ok(debug) = result { copy_debug = debug; } - clone(source, dest, CloneFallback::FSCopy) + clone(source, dest, CloneFallback::FSCopy, nofollow) } } (ReflinkMode::Auto, SparseMode::Auto) => { if source_is_stream { copy_debug.offload = OffloadReflinkDebug::Unsupported; - copy_stream(source, dest).map(|_| ()) + copy_stream(source, dest, nofollow).map(|_| ()) } else { let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_auto(source, dest); + let result = handle_reflink_auto_sparse_auto(source, dest, nofollow); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -346,9 +364,9 @@ pub(crate) fn copy_on_write( match copy_method { CopyMethod::SparseCopyWithoutHole => { - clone(source, dest, CloneFallback::SparseCopyWithoutHole) + clone(source, dest, CloneFallback::SparseCopyWithoutHole, nofollow) } - _ => clone(source, dest, CloneFallback::FSCopy), + _ => clone(source, dest, CloneFallback::FSCopy, nofollow), } } } @@ -357,7 +375,7 @@ pub(crate) fn copy_on_write( copy_debug.sparse_detection = SparseDebug::No; copy_debug.reflink = OffloadReflinkDebug::Yes; - clone(source, dest, CloneFallback::Error) + clone(source, dest, CloneFallback::Error, nofollow) } (ReflinkMode::Always, _) => { return Err(translate!("cp-error-reflink-always-sparse-auto").into()); @@ -372,6 +390,7 @@ pub(crate) fn copy_on_write( fn handle_reflink_auto_sparse_always( source: &Path, dest: &Path, + nofollow: bool, ) -> Result<(CopyDebug, CopyMethod), std::io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -379,8 +398,8 @@ fn handle_reflink_auto_sparse_always( sparse_detection: SparseDebug::Zeros, }; let mut copy_method = CopyMethod::Default; - let (data_flag, size, blocks) = check_for_data(source)?; - let sparse_flag = check_sparse_detection(source)?; + let (data_flag, size, blocks) = check_for_data(source, nofollow)?; + let sparse_flag = check_sparse_detection(source, nofollow)?; if data_flag || size < 512 { copy_debug.offload = OffloadReflinkDebug::Avoided; @@ -409,14 +428,17 @@ 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, + nofollow: bool, +) -> Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, sparse_detection: SparseDebug::No, }; - let (data_flag, size, _blocks) = check_for_data(source)?; - let sparse_flag = check_sparse_detection(source)?; + let (data_flag, size, _blocks) = check_for_data(source, nofollow)?; + let sparse_flag = check_sparse_detection(source, nofollow)?; if sparse_flag { copy_debug.sparse_detection = SparseDebug::SeekHole; @@ -430,15 +452,18 @@ fn handle_reflink_never_sparse_never(source: &Path) -> Result Result { +fn handle_reflink_auto_sparse_never( + source: &Path, + nofollow: bool, +) -> Result { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, sparse_detection: SparseDebug::No, }; - let (data_flag, size, _blocks) = check_for_data(source)?; - let sparse_flag = check_sparse_detection(source)?; + let (data_flag, size, _blocks) = check_for_data(source, nofollow)?; + let sparse_flag = check_sparse_detection(source, nofollow)?; if sparse_flag { copy_debug.sparse_detection = SparseDebug::SeekHole; @@ -455,6 +480,7 @@ fn handle_reflink_auto_sparse_never(source: &Path) -> Result Result<(CopyDebug, CopyMethod), std::io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -463,8 +489,8 @@ fn handle_reflink_auto_sparse_auto( }; let mut copy_method = CopyMethod::Default; - let (data_flag, size, blocks) = check_for_data(source)?; - let sparse_flag = check_sparse_detection(source)?; + let (data_flag, size, blocks) = check_for_data(source, nofollow)?; + let sparse_flag = check_sparse_detection(source, nofollow)?; if (data_flag && size != 0) || (size > 0 && size < 512) { copy_debug.offload = OffloadReflinkDebug::Yes; @@ -498,6 +524,7 @@ fn handle_reflink_auto_sparse_auto( fn handle_reflink_never_sparse_auto( source: &Path, dest: &Path, + nofollow: bool, ) -> Result<(CopyDebug, CopyMethod), std::io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -505,8 +532,8 @@ fn handle_reflink_never_sparse_auto( sparse_detection: SparseDebug::No, }; - let (data_flag, size, blocks) = check_for_data(source)?; - let sparse_flag = check_sparse_detection(source)?; + let (data_flag, size, blocks) = check_for_data(source, nofollow)?; + let sparse_flag = check_sparse_detection(source, nofollow)?; let mut copy_method = CopyMethod::Default; if data_flag || size < 512 { @@ -534,6 +561,7 @@ fn handle_reflink_never_sparse_auto( fn handle_reflink_never_sparse_always( source: &Path, dest: &Path, + nofollow: bool, ) -> Result<(CopyDebug, CopyMethod), std::io::Error> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, @@ -542,8 +570,8 @@ fn handle_reflink_never_sparse_always( }; let mut copy_method = CopyMethod::SparseCopy; - let (data_flag, size, blocks) = check_for_data(source)?; - let sparse_flag = check_sparse_detection(source)?; + let (data_flag, size, blocks) = check_for_data(source, nofollow)?; + let sparse_flag = check_sparse_detection(source, nofollow)?; if data_flag || size < 512 { copy_debug.offload = OffloadReflinkDebug::Avoided; diff --git a/src/uu/cp/src/platform/macos.rs b/src/uu/cp/src/platform/macos.rs index a545c552ba4..54e04b2bc1e 100644 --- a/src/uu/cp/src/platform/macos.rs +++ b/src/uu/cp/src/platform/macos.rs @@ -28,6 +28,7 @@ pub(crate) fn copy_on_write( sparse_mode: SparseMode, context: &str, source_is_stream: bool, + _nofollow: bool, ) -> CopyResult { if sparse_mode != SparseMode::Auto { return Err(translate!("cp-error-sparse-not-supported") @@ -69,8 +70,19 @@ pub(crate) fn copy_on_write( { // clonefile(2) fails if the destination exists. Remove it and try again. Do not // bother to check if removal worked because we're going to try to clone again. - // first lets make sure the dest file is not read only - if fs::metadata(dest).is_ok_and(|md| !md.permissions().readonly()) { + // first lets make sure the dest file is not read only. + // + // If dest is a symlink, GNU cp follows it and writes through to + // the target rather than replacing the link itself. Removing + // dest here would unlink the symlink and the retry would + // clonefile a regular file in its place. Skip the retry — the + // AlreadyExists error stays in `error` and we fall through to + // fs::copy below, which follows the symlink via O_TRUNC. + let dest_is_symlink = + fs::symlink_metadata(dest).is_ok_and(|md| md.file_type().is_symlink()); + if !dest_is_symlink + && fs::metadata(dest).is_ok_and(|md| !md.permissions().readonly()) + { // remove and copy again // TODO: rewrite this to better match linux behavior // linux first opens the source file and destination file then uses the file diff --git a/src/uu/cp/src/platform/other_unix.rs b/src/uu/cp/src/platform/other_unix.rs index 2db85c56af1..cfd3c716d21 100644 --- a/src/uu/cp/src/platform/other_unix.rs +++ b/src/uu/cp/src/platform/other_unix.rs @@ -3,12 +3,10 @@ // 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::os::unix::fs::OpenOptionsExt; use std::path::Path; use uucore::buf_copy; -use uucore::mode::get_umask; +use uucore::safe_copy::{create_dest_restrictive, open_source}; use uucore::translate; use crate::{ @@ -24,6 +22,7 @@ pub(crate) fn copy_on_write( sparse_mode: SparseMode, context: &str, source_is_stream: bool, + nofollow: bool, ) -> CopyResult { if reflink_mode != ReflinkMode::Never { return Err(translate!("cp-error-reflink-not-supported") @@ -42,13 +41,10 @@ pub(crate) fn copy_on_write( }; if source_is_stream { - let mut src_file = File::open(source)?; - let mode = 0o622 & !get_umask(); - let mut dst_file = OpenOptions::new() - .create(true) - .write(true) - .mode(mode) - .open(dest)?; + let mut src_file = open_source(source, nofollow) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + let mut dst_file = create_dest_restrictive(dest, false) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let dest_is_stream = is_stream(&dst_file.metadata()?); if !dest_is_stream { @@ -63,7 +59,16 @@ pub(crate) fn copy_on_write( return Ok(copy_debug); } - fs::copy(source, dest).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + // Replacement for fs::copy: restrictive 0o600 dest mode (#10011) and + // O_NOFOLLOW on the *source* under -P (#10017). The destination open + // intentionally does not use O_NOFOLLOW so that an existing symlink at + // dest is followed, matching GNU cp. + let mut src_file = + open_source(source, nofollow).map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + let mut dst_file = create_dest_restrictive(dest, false) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; + std::io::copy(&mut src_file, &mut dst_file) + .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; Ok(copy_debug) } diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index d9683264652..2b42edaa349 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.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 getxattr posix_acl_default +// spell-checker:ignore getxattr posix_acl_default posix_acl_access //! Set of functions to manage xattr on files and dirs use itertools::Itertools; @@ -45,6 +45,28 @@ pub fn copy_xattrs_skip_selinux>(source: P, dest: P) -> std::io:: Ok(()) } +/// Copies only the POSIX ACL xattrs (`system.posix_acl_access` and +/// `system.posix_acl_default`) from `source` to `dest`. +/// +/// GNU `cp -p` preserves ACLs as part of mode preservation but does not +/// preserve other (user/security) xattrs unless `--preserve=xattr` or +/// `-a` is requested. On Linux, POSIX ACLs are stored as the two `system.*` +/// xattrs above; copying them here without copying the rest gives the +/// GNU-compatible "preserve mode (incl. ACLs) but not user xattrs" behavior. +/// +/// Errors from the underlying xattr calls are silently ignored: filesystems +/// without ACL/xattr support are common, and GNU cp itself does not surface +/// failures here when `mode` is the only thing being preserved. +#[cfg(unix)] +pub fn copy_acls>(source: P, dest: P) { + for name in ["system.posix_acl_access", "system.posix_acl_default"] { + if let Ok(Some(value)) = xattr::get(&source, name) { + // Best-effort: silently skip if dest doesn't support ACL xattrs. + let _ = xattr::set(&dest, name, &value); + } + } +} + /// Retrieves the extended attributes (xattrs) of a given file or directory. /// /// # Arguments diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 28af3075eef..d69ca1a0648 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.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 (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile uufs xattrs +// spell-checker:ignore (flags) reflink (fs) tmpfs (linux) rlimit Rlim NOFILE clob btrfs neve ROOTDIR USERDIR outfile uufs xattrs ELOOP // spell-checker:ignore bdfl hlsl IRWXO IRWXG nconfined matchpathcon libselinux-devel prwx doesnotexist reftests subdirs mksocket srwx #[cfg(unix)] use rstest::rstest; @@ -1768,6 +1768,47 @@ fn test_cp_preserve_all() { } } +// GNU `cp -p` preserves mode, ownership, and timestamps but NOT xattrs. +// xattr preservation requires explicit `--preserve=xattr` or `-a`. See #9704. +#[test] +#[cfg(all( + unix, + not(any(target_os = "android", target_os = "openbsd", target_os = "macos")) +))] +fn test_cp_p_does_not_preserve_xattr_by_default() { + use std::process::Command; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("src"); + + let xattr_key = "user.test_preserve_p"; + let setfattr = Command::new("setfattr") + .args(["-n", xattr_key, "-v", "v", &at.plus_as_string("src")]) + .status(); + match setfattr { + Ok(s) if s.success() => {} + _ => { + println!("test skipped: setfattr not available / filesystem rejects xattrs"); + return; + } + } + + scene + .ucmd() + .args(&["-p", &at.plus_as_string("src"), &at.plus_as_string("dst")]) + .succeeds(); + + let out = Command::new("getfattr") + .args(["--only-values", "-n", xattr_key, &at.plus_as_string("dst")]) + .output() + .expect("getfattr failed"); + assert!( + !out.status.success(), + "cp -p should not preserve xattrs by default, but '{xattr_key}' was copied" + ); +} + #[test] #[cfg(all(unix, not(any(target_os = "android", target_os = "openbsd"))))] fn test_cp_preserve_xattr() { @@ -3036,8 +3077,6 @@ fn test_cp_symlink_overwrite_detection() { .fails() .stderr_only(if cfg!(target_os = "windows") { "cp: will not copy 'good/README' through just-created symlink 'tmp\\README'\n" - } else if cfg!(target_os = "macos") { - "cp: will not overwrite just-created 'tmp/README' with 'good/README'\n" } else { "cp: will not copy 'good/README' through just-created symlink 'tmp/README'\n" }); @@ -7859,6 +7898,28 @@ fn test_cp_preserve_context_with_z_fails() { .stderr_contains("cannot combine"); } +// Covers the happy path for issue #9750: when chown succeeds (src owner == +// current user), `cp -p` preserves setuid/setgid. The failure-path behavior — +// stripping setuid/setgid when chown cannot preserve ownership — requires a +// multi-user setup (source owned by a different uid, cp run as non-root) and +// is exercised by GNU's test suite; documenting here as future coverage. +#[test] +#[cfg(unix)] +fn test_cp_preserve_setuid_when_chown_succeeds() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("src"); + at.set_mode("src", 0o4755); + + ucmd.arg("-p").arg("src").arg("dst").succeeds(); + + let mode = at.metadata("dst").mode() & 0o7777; + assert_eq!( + mode & 0o4000, + 0o4000, + "setuid bit should be preserved when chown succeeds (got mode {mode:o})" + ); +} + #[test] #[cfg(all(unix, not(target_os = "macos")))] fn test_cp_recursive_non_utf8_source() { @@ -7877,3 +7938,114 @@ fn test_cp_recursive_non_utf8_source() { assert!(at.plus("dir2").join("a").exists()); } + +// Regression guard for issue #10011: cp now creates the destination with +// mode 0o600 instead of the umask-derived 0o666, so another user in a +// shared directory cannot open the file through its permissive initial +// mode before cp applies the final permissions. The final mode must still +// match the source mode masked by the running umask, so no user-visible +// behavior changes. +#[test] +#[cfg(unix)] +fn test_cp_final_mode_unchanged_after_restrictive_create() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("src"); + at.set_mode("src", 0o644); + + ucmd.umask(0o022).arg("src").arg("dst").succeeds(); + + let mode = at.metadata("dst").mode() & 0o777; + assert_eq!( + mode, 0o644, + "dst final mode should match source & ~umask (got {mode:o})" + ); +} + +// Sanity check for the `-P` happy path: a symlink source is copied as a +// symlink, not by following it. The actual `O_NOFOLLOW` invariant for +// issue #10017 (path swap to a symlink between lstat and open) cannot be +// raced deterministically from a unit test; that is locked in by the +// strace check in util/check-safe-traversal.sh, which fails if a future +// change drops `O_NOFOLLOW` from the source open under `-P`. +#[test] +#[cfg(unix)] +fn test_cp_no_dereference_copies_symlink_as_symlink() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("target", "secret target contents"); + at.symlink_file("target", "src_link"); + + ucmd.arg("-P").arg("src_link").arg("dst").succeeds(); + assert!(at.symlink_exists("dst")); + assert!(at.read_symlink("dst").ends_with("target")); +} + +// Regression for GNU tests/cp/deref-slink: when the destination exists as +// a symlink, `cp -d` (which implies --no-dereference for the source) must +// still follow the destination symlink and overwrite the link's target. +// `-P`/`-d` only forbids dereferencing on the source side; applying +// O_NOFOLLOW to the dest open broke this and surfaced as ELOOP. +#[test] +#[cfg(unix)] +fn test_cp_d_overwrites_existing_symlink_dest() { + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("f"); + at.touch("slink-target"); + at.symlink_file("slink-target", "slink"); + + ucmd.arg("-d").arg("f").arg("slink").succeeds(); + + // The destination symlink itself remains a symlink (GNU follows it + // through to the target rather than replacing it). + assert!(at.symlink_exists("slink")); + assert!(at.read_symlink("slink").ends_with("slink-target")); +} + +// Regression for GNU tests/cp/acl: `cp -p` must preserve POSIX ACLs on +// Linux. ACLs are part of GNU's `mode` preservation, not its `xattr` +// preservation, so the default-no-xattr change in #9704 must not strip +// them. Only runs when `setfacl` is available so non-ACL filesystems and +// non-Linux CI do not flag spurious failures. +#[test] +#[cfg(target_os = "linux")] +fn test_cp_p_preserves_posix_acls() { + use std::process::Command; + + if Command::new("setfacl").arg("--version").output().is_err() { + return; + } + if Command::new("getfacl").arg("--version").output().is_err() { + return; + } + + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("src"); + + let setfacl = Command::new("setfacl") + .arg("-m") + .arg("user:bin:rw-") + .arg(at.plus("src")) + .status(); + let Ok(status) = setfacl else { return }; + if !status.success() { + // Filesystem doesn't support ACLs; skip. + return; + } + + ucmd.arg("-p").arg("src").arg("dst").succeeds(); + + let src_acl = Command::new("getfacl") + .arg("--omit-header") + .arg(at.plus("src")) + .output() + .unwrap(); + let dst_acl = Command::new("getfacl") + .arg("--omit-header") + .arg(at.plus("dst")) + .output() + .unwrap(); + assert_eq!( + String::from_utf8_lossy(&src_acl.stdout), + String::from_utf8_lossy(&dst_acl.stdout), + "cp -p must preserve POSIX ACLs (GNU tests/cp/acl regression)", + ); +} diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh index 0462f6b9ef8..4a5552199a3 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -159,7 +159,7 @@ if [ "$USE_MULTICALL" -eq 1 ]; then AVAILABLE_UTILS=$($COREUTILS_BIN --list) else AVAILABLE_UTILS="" - for util in rm chmod chown chgrp du mv; do + for util in rm chmod chown chgrp du mv cp; do if [ -f "$PROJECT_ROOT/target/${PROFILE}/$util" ]; then AVAILABLE_UTILS="$AVAILABLE_UTILS $util" fi @@ -219,6 +219,42 @@ if echo "$AVAILABLE_UTILS" | grep -q "mv"; then check_utility "mv" "openat,renameat,newfstatat,rename" "openat" "test_mv_src test_mv_dst" "move_directory" fi +# cp invariant checks. Both #10011 (restrictive 0600 destination mode) and +# #10017 (O_NOFOLLOW on the -P source) need to hold; verify each on its own +# strace. +if echo "$AVAILABLE_UTILS" | grep -q "cp"; then + if [ "$USE_MULTICALL" -eq 1 ]; then + cp_cmd="$COREUTILS_BIN cp" + else + cp_cmd="$PROJECT_ROOT/target/${PROFILE}/cp" + fi + + # #10011: destination created with mode 0600 so other users cannot open + # the file through its umask-derived initial mode before cp narrows it. + echo "cp_perm_test" > test_cp_src_perm + rm -f test_cp_dst_perm + strace -f -e trace=openat -o strace_cp_dest_perm.log \ + $cp_cmd test_cp_src_perm test_cp_dst_perm 2>/dev/null || true + if ! grep -qE 'openat\(AT_FDCWD, "test_cp_dst_perm".*O_CREAT.*, 0600\)' strace_cp_dest_perm.log; then + cat strace_cp_dest_perm.log + fail_immediately "cp must create the destination with mode 0600 (issue #10011)" + fi + echo "✓ cp creates destination with restrictive 0600 mode" + rm -f test_cp_src_perm test_cp_dst_perm + + # #10017: -P opens source with O_NOFOLLOW so a path swap to a symlink + # between the lstat check and the open cannot redirect the copy. + echo "cp_nofollow_test" > test_cp_src + strace -f -e trace=openat -o strace_cp_nofollow.log \ + $cp_cmd -P test_cp_src test_cp_dst 2>/dev/null || true + if ! grep -qE 'openat\(AT_FDCWD, "test_cp_src".*O_NOFOLLOW' strace_cp_nofollow.log; then + cat strace_cp_nofollow.log + fail_immediately "cp -P must open the source with O_NOFOLLOW (issue #10017)" + fi + echo "✓ cp -P opens source with O_NOFOLLOW" + rm -f test_cp_src test_cp_dst +fi + echo "" echo "✓ Basic safe traversal verification completed" echo ""