chown/chgrp/chmod/rm: open recursive descent with O_NOFOLLOW

src/uu/chmod/src/chmod.rs

@@ -533,9 +533,12 @@ impl Chmoder {
                     )
                     .and(r);
 
-                // Recurse into subdirectories using the existing directory fd
+                // Recurse into subdirectories using the existing directory fd.
+                // Open with NoFollow unless `-L` was requested: this prevents a
+                // TOCTOU where an attacker swaps the just-stat'd directory for a
+                // symlink before the open and redirects the descent off-tree.
                 if meta.is_dir() {
-                    match dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow) {
+                    match dir_fd.open_subdir(&entry_name, should_follow_symlink.into()) {
                         Ok(child_dir_fd) => {
                             r = self.safe_traverse_dir(&child_dir_fd, &entry_path).and(r);
                         }

src/uu/rm/src/platform/unix.rs

@@ -380,8 +380,12 @@ pub fn safe_remove_dir_recursive_impl(path: &Path, dir_fd: &DirFd, options: &Opt
                 continue;
             }
 
-            // Recursively remove subdirectory using safe traversal
-            let child_dir_fd = match dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow) {
+            // Recursively remove subdirectory using safe traversal. rm never
+            // follows symlinks during recursion, so open with NoFollow: if an
+            // attacker swaps this just-stat'd directory for a symlink before the
+            // open, O_NOFOLLOW makes openat fail instead of descending off-tree
+            // and deleting unrelated files.
+            let child_dir_fd = match dir_fd.open_subdir(&entry_name, SymlinkBehavior::NoFollow) {
                 Ok(fd) => fd,
                 Err(e) => {
                     // If we can't open the subdirectory for safe traversal,

src/uucore/src/lib/features/perms.rs

@@ -536,9 +536,13 @@ impl ChownExecutor {
                 );
             }
 
-            // Recurse into subdirectories
+            // Recurse into subdirectories. Open with the same symlink behavior
+            // used for the stat above: with NoFollow (the default, `-P`/`-H`) an
+            // attacker that swaps the just-stat'd directory for a symlink between
+            // the stat and this open cannot redirect the descent off-tree
+            // (O_NOFOLLOW makes openat fail). Only follow when `-L` was requested.
             if meta.is_dir() && (follow || !meta.file_type().is_symlink()) {
-                match dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow) {
+                match dir_fd.open_subdir(&entry_name, follow.into()) {
                     Ok(subdir_fd) => {
                         self.safe_traverse_dir(&subdir_fd, &entry_path, ret);
                     }

src/uucore/src/lib/features/safe_traversal.rs

@@ -948,6 +948,35 @@ mod tests {
         assert!(subdir_fd.as_raw_fd() >= 0);
     }
 
+    #[test]
+    fn test_dirfd_open_subdir_nofollow_refuses_symlink() {
+        // A symlink to a directory must NOT be opened as a subdir when NoFollow
+        // is requested. Recursive chown/chgrp/chmod/rm rely on this to refuse a
+        // directory that was swapped for a symlink mid-traversal (TOCTOU),
+        // instead of following it off-tree.
+        let temp_dir = TempDir::new().unwrap();
+        let real_dir = temp_dir.path().join("real");
+        fs::create_dir(&real_dir).unwrap();
+        symlink(&real_dir, temp_dir.path().join("link")).unwrap();
+
+        let parent_fd = DirFd::open(temp_dir.path(), SymlinkBehavior::Follow).unwrap();
+
+        // NoFollow must reject the symlink (ELOOP).
+        let nofollow = parent_fd.open_subdir(OsStr::new("link"), SymlinkBehavior::NoFollow);
+        assert!(nofollow.is_err());
+
+        // Follow still resolves it (the explicit `-L` opt-in).
+        let follow = parent_fd.open_subdir(OsStr::new("link"), SymlinkBehavior::Follow);
+        assert!(follow.is_ok());
+
+        // A real directory is opened fine either way.
+        assert!(
+            parent_fd
+                .open_subdir(OsStr::new("real"), SymlinkBehavior::NoFollow)
+                .is_ok()
+        );
+    }
+
     #[test]
     fn test_dirfd_open_nonexistent_subdir() {
         let temp_dir = TempDir::new().unwrap();

util/check-safe-traversal.sh

@@ -168,6 +168,24 @@ check_utility() {
     fi
 }
 
+# Recursive descent must open every subdirectory relative to the parent dirfd
+# WITH O_NOFOLLOW. Otherwise an attacker who swaps the just-stat'd directory for
+# a symlink between the stat and the open redirects the descent off-tree (the
+# chown/chgrp/chmod/rm recursive-descent TOCTOU). The top-level command-line
+# argument is opened via AT_FDCWD and is allowed to follow, so it is excluded by
+# matching only numeric-dirfd openat calls.
+assert_descent_nofollow() {
+    local util="$1"
+    local log="$2"
+    local offenders
+    offenders=$(grep -E 'openat\([0-9]+, "[^"]*",[^)]*O_DIRECTORY' "$log" | grep -v 'O_NOFOLLOW' || true)
+    if [ -n "$offenders" ]; then
+        echo "$offenders"
+        fail_immediately "$util descends into subdirectories without O_NOFOLLOW (TOCTOU symlink-swap risk)"
+    fi
+    echo "✓ $util descent opens use O_NOFOLLOW"
+}
+
 # Get list of available utilities
 if [ "$USE_MULTICALL" -eq 1 ]; then
     AVAILABLE_UTILS=$($COREUTILS_BIN --list)
@@ -192,6 +210,7 @@ if echo "$AVAILABLE_UTILS" | grep -q "rm"; then
     if grep -qE 'statx\(AT_FDCWD, "[^"]*/' strace_rm_recursive_remove.log; then
         fail_immediately "rm is using path-based statx (multi-component relative path); expected dirfd-relative newfstatat"
     fi
+    assert_descent_nofollow "rm" strace_rm_recursive_remove.log
 fi
 
 # Test chmod - should use openat, fchmodat, newfstatat
@@ -203,6 +222,7 @@ if echo "$AVAILABLE_UTILS" | grep -q "chmod"; then
     if grep -q 'openat(AT_FDCWD, "test_chmod/' strace_chmod_recursive_chmod.log; then
         fail_immediately "chmod recursed using AT_FDCWD with a multi-component path; expected dirfd-relative openat"
     fi
+    assert_descent_nofollow "chmod" strace_chmod_recursive_chmod.log
 fi
 
 # Test chown - should use openat, fchownat, newfstatat
@@ -211,12 +231,14 @@ if echo "$AVAILABLE_UTILS" | grep -q "chown"; then
     USER_ID=$(id -u)
     GROUP_ID=$(id -g)
     check_utility "chown" "openat,fchownat,newfstatat,chown,lchown" "openat fchownat" "-R $USER_ID:$GROUP_ID test_chown" "recursive_chown"
+    assert_descent_nofollow "chown" strace_chown_recursive_chown.log
 fi
 
 # Test chgrp - should use openat, fchownat, newfstatat
 if echo "$AVAILABLE_UTILS" | grep -q "chgrp"; then
     cp -r test_dir test_chgrp
     check_utility "chgrp" "openat,fchownat,newfstatat,chown,lchown" "openat fchownat" "-R $GROUP_ID test_chgrp" "recursive_chgrp"
+    assert_descent_nofollow "chgrp" strace_chgrp_recursive_chgrp.log
 fi
 
 # Test du - should use openat, newfstatat