diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index a50f0a6bf0e..6eb9add77ed 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -519,9 +519,18 @@ impl Chmoder { .handle_symlink_during_safe_recursion(&entry_path, dir_fd, &entry_name) .and(r); } else { - // For regular files and directories, chmod them + // For regular files and directories, chmod them. + // Always use NoFollow: we already confirmed via stat that the entry + // is not a symlink, so this prevents TOCTOU races where an attacker + // replaces the entry with a symlink between stat and chmod. r = self - .safe_chmod_file(&entry_path, dir_fd, &entry_name, meta.mode() & 0o7777) + .safe_chmod_file( + &entry_path, + dir_fd, + &entry_name, + meta.mode() & 0o7777, + SymlinkBehavior::NoFollow, + ) .and(r); // Recurse into subdirectories using the existing directory fd @@ -555,13 +564,23 @@ impl Chmoder { // During recursion, determine behavior based on traversal mode match self.traverse_symlinks { TraverseSymlinks::All => { - // Follow all symlinks during recursion + // Follow all symlinks during recursion. + // NOTE: This path-based stat + Follow is only reachable when the user + // explicitly specifies -L (--dereference). In that case the user has + // consciously opted in to following symlinks, so a path-based stat + // followed by Follow is the intended behavior and not a TOCTOU concern. // Check if the symlink target is a directory, but handle dangling symlinks gracefully match fs::metadata(path) { Ok(meta) if meta.is_dir() => self.walk_dir_with_context(path, false), Ok(meta) => { // It's a file symlink, chmod it using safe traversal - self.safe_chmod_file(path, dir_fd, entry_name, meta.mode() & 0o7777) + self.safe_chmod_file( + path, + dir_fd, + entry_name, + meta.mode() & 0o7777, + self.dereference.into(), + ) } Err(_) => { // Dangling symlink, chmod it without dereferencing @@ -584,13 +603,13 @@ impl Chmoder { dir_fd: &DirFd, entry_name: &std::ffi::OsStr, current_mode: u32, + symlink_behavior: SymlinkBehavior, ) -> UResult<()> { // Calculate the new mode using the helper method let (new_mode, _) = self.calculate_new_mode(current_mode, file_path.is_dir())?; // Use safe traversal to change the mode - let follow_symlinks = self.dereference; - if let Err(_e) = dir_fd.chmod_at(entry_name, new_mode, follow_symlinks.into()) { + if let Err(_e) = dir_fd.chmod_at(entry_name, new_mode, symlink_behavior) { if self.verbose { println!( "failed to change mode of {} to {new_mode:o}", diff --git a/src/uucore/src/lib/features/safe_traversal.rs b/src/uucore/src/lib/features/safe_traversal.rs index 98fc81dd928..820cf12fad9 100644 --- a/src/uucore/src/lib/features/safe_traversal.rs +++ b/src/uucore/src/lib/features/safe_traversal.rs @@ -10,7 +10,7 @@ // // spell-checker:ignore CLOEXEC RDONLY TOCTOU closedir dirp fdopendir fstatat openat REMOVEDIR unlinkat smallfile // spell-checker:ignore RAII dirfd fchownat fchown FchmodatFlags fchmodat fchmod mkdirat CREAT WRONLY ELOOP ENOTDIR -// spell-checker:ignore atimensec mtimensec ctimensec +// spell-checker:ignore atimensec mtimensec ctimensec opath chmods #[cfg(test)] use std::os::unix::ffi::OsStringExt; @@ -293,21 +293,121 @@ impl DirFd { mode: u32, symlink_behavior: SymlinkBehavior, ) -> io::Result<()> { + let name_cstr = + CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + + // --- fchmodat2 path (Linux 6.6+, asm-generic arches only) --- + // Uses the raw mode value directly; no nix::Mode conversion needed. + // Only enabled on asm-generic architectures where syscall number 452 is + // correct (x86_64, x86, arm, aarch64, riscv). MIPS/SPARC/PowerPC/Alpha + // use different numbering and are not supported until libc exposes + // SYS_fchmodat2 for them. + #[cfg(all( + target_os = "linux", + any( + target_arch = "x86_64", + target_arch = "x86", + target_arch = "arm", + target_arch = "aarch64", + target_arch = "riscv64", + target_arch = "riscv32", + ), + ))] + if matches!(symlink_behavior, SymlinkBehavior::NoFollow) { + use std::sync::atomic::{AtomicBool, Ordering}; + + // Cache: if fchmodat2 returned ENOSYS once, the kernel is too old + // and will never support it. Skip the syscall on subsequent calls. + static FCHMODAT2_UNAVAILABLE: AtomicBool = AtomicBool::new(false); + + if !FCHMODAT2_UNAVAILABLE.load(Ordering::Relaxed) { + // Syscall number for fchmodat2 on asm-generic architectures. + const SYS_FCHMODAT2: libc::c_long = 452; + // SAFETY: syscall(2) is an FFI call. We pass valid arguments: + // - fd: valid open file descriptor + // - name: valid C string pointer (name_cstr lives for the duration) + // - mode: valid mode_t value + // - flags: AT_SYMLINK_NOFOLLOW (valid flag for fchmodat2) + let res = unsafe { + libc::syscall( + SYS_FCHMODAT2, + self.fd.as_raw_fd(), + name_cstr.as_ptr(), + mode as libc::mode_t, + libc::AT_SYMLINK_NOFOLLOW, + ) + }; + if res == 0 { + return Ok(()); + } + let err = io::Error::last_os_error(); + match err.raw_os_error() { + Some(libc::ENOSYS) => { + FCHMODAT2_UNAVAILABLE.store(true, Ordering::Relaxed); + // Fall through to fchmodat + } + _ => return Err(err), + } + } + } + + // --- fchmodat fallback path --- + // nix::Mode conversion is needed here because fchmodat() requires it. + let nix_mode = Mode::from_bits_truncate(mode as libc::mode_t); + let flags = if symlink_behavior.should_follow() { FchmodatFlags::FollowSymlink } else { FchmodatFlags::NoFollowSymlink }; - let mode = Mode::from_bits_truncate(mode as libc::mode_t); + match fchmodat(&self.fd, name_cstr.as_c_str(), nix_mode, flags) { + Ok(()) => Ok(()), + Err(e) + if !symlink_behavior.should_follow() + && (e == nix::errno::Errno::EOPNOTSUPP || e == nix::errno::Errno::ENOTSUP) => + { + // musl does not emulate AT_SYMLINK_NOFOLLOW via /proc/self/fd + // like glibc does, so fchmodat returns EOPNOTSUPP on old kernels. + // Fall back to O_PATH + /proc/self/fd/{fd} + fchmod. + #[cfg(target_os = "linux")] + { + self.chmod_at_via_opath(name_cstr.as_c_str(), mode) + } + #[cfg(not(target_os = "linux"))] + { + Err(io::Error::from_raw_os_error(e as i32)) + } + } + Err(e) => Err(io::Error::from_raw_os_error(e as i32)), + } + } - let name_cstr = - CString::new(name.as_bytes()).map_err(|_| SafeTraversalError::PathContainsNull)?; + /// O_PATH-based fallback for chmod when fchmodat with AT_SYMLINK_NOFOLLOW + /// is not available (musl on kernel < 6.6). + /// + /// Opens the file with O_PATH|O_NOFOLLOW to get an fd without following + /// symlinks, then chmods via /proc/self/fd/{fd}. This avoids the TOCTOU + /// race because the fd pins the inode. + /// + #[cfg(target_os = "linux")] + fn chmod_at_via_opath(&self, name: &std::ffi::CStr, mode: u32) -> io::Result<()> { + use rustix::fs::{Mode, OFlags, chmod, openat}; - fchmodat(&self.fd, name_cstr.as_c_str(), mode, flags) - .map_err(|e| io::Error::from_raw_os_error(e as i32))?; + let fd = openat( + &self.fd, + name, + OFlags::PATH | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|e| io::Error::from_raw_os_error(e.raw_os_error()))?; - Ok(()) + let proc_path = format!("/proc/self/fd/{}\0", fd.as_raw_fd()); + let proc_cstr = std::ffi::CStr::from_bytes_with_nul(proc_path.as_bytes()) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "invalid proc path"))?; + + chmod(proc_cstr, Mode::from_bits_truncate(mode)) + .map_err(|e| io::Error::from_raw_os_error(e.raw_os_error())) } /// Change mode of this directory @@ -800,6 +900,7 @@ impl std::os::unix::fs::MetadataExt for Metadata { mod tests { use super::*; use std::fs; + use std::os::unix::fs::MetadataExt; use std::os::unix::fs::symlink; use std::os::unix::io::IntoRawFd; use tempfile::TempDir; @@ -1227,6 +1328,43 @@ mod tests { assert!(result_nofollow.is_err()); } + /// Verify that chmod_at with NoFollow does not change the symlink target's mode. + /// This test demonstrates that the TOCTOU race in recursive chmod is closed: + /// chmod on a symlink entry should not affect the target file. + #[test] + fn test_chmod_at_nofollow_preserves_target_mode() { + let temp_dir = TempDir::new().unwrap(); + + // Create a sentinel file outside the traversal directory + let sentinel = temp_dir.path().join("sentinel"); + fs::write(&sentinel, "victim").unwrap(); + let sentinel_mode = fs::symlink_metadata(&sentinel).unwrap().mode(); + + // Create a subdirectory with a symlink pointing to the sentinel + let subdir = temp_dir.path().join("subdir"); + fs::create_dir(&subdir).unwrap(); + let link = subdir.join("link"); + symlink(&sentinel, &link).unwrap(); + + // Open the subdirectory and chmod the symlink entry with NoFollow + let dir_fd = DirFd::open(&subdir, SymlinkBehavior::Follow).unwrap(); + let result = dir_fd.chmod_at(OsStr::new("link"), 0o777, SymlinkBehavior::NoFollow); + + // On Linux 6.6+ (fchmodat2), the chmod should succeed without affecting the target. + // On older kernels, fchmodat with AT_SYMLINK_NOFOLLOW returns EOPNOTSUPP/ENOTSUP, + // which is acceptable — the important thing is the target is NOT modified. + if let Ok(()) = result { + // fchmodat2 succeeded: verify sentinel mode is unchanged + let new_sentinel_mode = fs::symlink_metadata(&sentinel).unwrap().mode(); + assert_eq!( + new_sentinel_mode, sentinel_mode, + "sentinel mode should not change when chmod'ing symlink with NoFollow" + ); + } + // If result is Err (EOPNOTSUPP on old kernels), the target is also unchanged, + // which is the correct behavior — no silent modification. + } + #[test] fn test_open_nofollow_fails_on_symlink() { let temp_dir = TempDir::new().unwrap(); diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh index 0462f6b9ef8..799996cfa59 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -81,12 +81,26 @@ check_utility() { # Check for expected safe syscalls local found_safe=0 for syscall in $expected_syscalls; do - if grep -q "$syscall" "$strace_log"; then - echo "✓ Found $syscall() (safe traversal)" - found_safe=$((found_safe + 1)) - else - fail_immediately "Missing $syscall() (safe traversal not active for $util)" - fi + # fchmodat2 is the modern replacement for fchmodat on Linux 6.6+ + # Accept either as a valid safe traversal syscall + case "$syscall" in + fchmodat) + if grep -qE "fchmodat2?\(" "$strace_log"; then + echo "✓ Found fchmodat/fchmodat2() (safe traversal)" + found_safe=$((found_safe + 1)) + else + fail_immediately "Missing fchmodat() or fchmodat2() (safe traversal not active for $util)" + fi + ;; + *) + if grep -q "$syscall" "$strace_log"; then + echo "✓ Found $syscall() (safe traversal)" + found_safe=$((found_safe + 1)) + else + fail_immediately "Missing $syscall() (safe traversal not active for $util)" + fi + ;; + esac done # Count detailed syscall statistics @@ -95,7 +109,7 @@ check_utility() { openat_count=$(grep -c "openat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") unlinkat_count=$(grep -c "unlinkat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") - fchmodat_count=$(grep -c "fchmodat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") + fchmodat_count=$(grep -cE "fchmodat2?\(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") fchownat_count=$(grep -c "fchownat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") newfstatat_count=$(grep -c "newfstatat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") renameat_count=$(grep -c "renameat(" "$strace_log" 2>/dev/null | tr -d '\n' || echo "0") @@ -183,7 +197,7 @@ fi # Test chmod - should use openat, fchmodat, newfstatat if echo "$AVAILABLE_UTILS" | grep -q "chmod"; then cp -r test_dir test_chmod - check_utility "chmod" "openat,fchmodat,newfstatat,chmod" "openat fchmodat" "-R 755 test_chmod" "recursive_chmod" + check_utility "chmod" "openat,fchmodat,fchmodat2,newfstatat,chmod" "openat fchmodat" "-R 755 test_chmod" "recursive_chmod" # Additional regression guard: ensure recursion uses dirfd-relative openat, not AT_FDCWD with a multi-component path if grep -q 'openat(AT_FDCWD, "test_chmod/' strace_chmod_recursive_chmod.log; then