uutils/coreutils
0
0
Fork 0
mirror of https://github.com/uutils/coreutils.git synced 2026-06-10 15:48:22 -07:00
Code Issues Packages Projects Releases Wiki Activity

chmod: fix TOCTOU race in recursive traversal (#11918)

Browse Source 
* chmod: fix TOCTOU race in recursive traversal

Use fchmodat2 (Linux 6.6+) with AT_SYMLINK_NOFOLLOW to prevent an
attacker from replacing a directory entry with a symlink between the
stat and chmod calls. Falls back to fchmodat on older kernels.

- Restrict fchmodat2 (syscall 452) to asm-generic architectures only
  (x86_64, x86, arm, aarch64, riscv)
- Add SAFETY comment on unsafe syscall block per project convention
- Add O_PATH + /proc/self/fd fallback for musl on kernel < 6.6
- Cache ENOSYS result with AtomicBool to skip fchmodat2 on old kernels
- Remove unnecessary nix::Mode round-trip on the fchmodat2 path
- safe_chmod_file() takes explicit SymlinkBehavior parameter
- Always pass NoFollow for regular entries during recursion
- Document residual TOCTOU in symlink branch as intentional for -L
- Add test verifying NoFollow chmod doesn't modify symlink target
- Update check-safe-traversal.sh to recognize fchmodat2
This commit is contained in:
mattsu
2026-05-06 05:06:06 +09:00
committed by GitHub
parent fb93f53b3a
commit b195b1de88
3 changed files with 192 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/uu/chmod/src/chmod.rs
+25 -6
View File
@@ -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}",
src/uucore/src/lib/features/safe_traversal.rs
+145 -7
View File
@@ -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
@@ -798,6 +898,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();
util/check-safe-traversal.sh
+22 -8
View File
@@ -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