chmod: on linux use the safe traversal functions

Author: sylvestre

src/uu/chmod/src/chmod.rs

@@ -456,35 +456,70 @@ impl Chmoder {
 
     #[cfg(target_os = "linux")]
     fn walk_dir_with_context(&self, file_path: &Path, is_command_line_arg: bool) -> UResult<()> {
-        let mut r = self.chmod_file(file_path);
-
         // Determine whether to traverse symlinks based on context and traversal mode
         let should_follow_symlink = match self.traverse_symlinks {
             TraverseSymlinks::All => true,
             TraverseSymlinks::First => is_command_line_arg, // Only follow symlinks that are command line args
             TraverseSymlinks::None => false,
         };
 
-        // If the path is a directory (or we should follow symlinks), recurse into it using safe traversal
-        if (!file_path.is_symlink() || should_follow_symlink) && file_path.is_dir() {
+        // Use safe syscalls for the root directory as well to prevent TOCTOU attacks
+        if file_path.is_dir() && (!file_path.is_symlink() || should_follow_symlink) {
+            // For directories, use safe traversal from the start
             match DirFd::open(file_path) {
                 Ok(dir_fd) => {
-                    r = self.safe_traverse_dir(&dir_fd, file_path).and(r);
+                    // First chmod the directory itself using fchmod (safe)
+                    let dir_result = self.safe_chmod_dir(&dir_fd, file_path);
+                    // Then traverse its contents
+                    let traverse_result = self.safe_traverse_dir(&dir_fd, file_path);
+                    dir_result.and(traverse_result)
                 }
                 Err(err) => {
                     // Handle permission denied errors with proper file path context
                     if err.kind() == std::io::ErrorKind::PermissionDenied {
-                        r = r.and(Err(ChmodError::PermissionDenied(
-                            file_path.to_string_lossy().to_string(),
+                        Err(
+                            ChmodError::PermissionDenied(file_path.to_string_lossy().to_string())
+                                .into(),
                         )
-                        .into()));
                     } else {
-                        r = r.and(Err(err.into()));
+                        Err(err.into())
                     }
                 }
             }
+        } else {
+            // For non-directories (files, symlinks), use the regular chmod_file method
+            self.chmod_file(file_path)
         }
-        r
+    }
+
+    #[cfg(target_os = "linux")]
+    fn safe_chmod_dir(&self, dir_fd: &DirFd, file_path: &Path) -> UResult<()> {
+        // Get the current mode using fstat (safe)
+        let stat = dir_fd
+            .fstat()
+            .map_err(|_e| ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()))?;
+
+        let current_mode = stat.st_mode;
+        // Calculate the new mode using the helper method
+        let (new_mode, _) = self.calculate_new_mode(current_mode, true)?; // true = is_dir
+
+        // Use fchmod (safe) to change the directory's mode
+        if let Err(_e) = dir_fd.fchmod(new_mode) {
+            if self.verbose {
+                println!(
+                    "failed to change mode of {} to {:o}",
+                    file_path.quote(),
+                    new_mode
+                );
+            }
+            return Err(
+                ChmodError::PermissionDenied(file_path.to_string_lossy().to_string()).into(),
+            );
+        }
+
+        // Report the change if verbose
+        self.report_permission_change(file_path, current_mode, new_mode);
+        Ok(())
     }
 
     #[cfg(target_os = "linux")]

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

@@ -307,14 +307,41 @@ impl ChownExecutor {
         }
 
         let ret = if self.matched(meta.uid(), meta.gid()) {
-            match wrap_chown(
+            // Use safe syscalls for root directory to prevent TOCTOU attacks
+            #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+            let chown_result = if path.is_dir() {
+                // For directories, use safe traversal from the start
+                match DirFd::open(path) {
+                    Ok(dir_fd) => self.safe_chown_dir(&dir_fd, path, &meta),
+                    Err(e) => Err(format!(
+                        "cannot access '{}': {}",
+                        path.display(),
+                        strip_errno(&e)
+                    )),
+                }
+            } else {
+                // For non-directories (files, symlinks), use the regular wrap_chown method
+                wrap_chown(
+                    path,
+                    &meta,
+                    self.dest_uid,
+                    self.dest_gid,
+                    self.dereference,
+                    self.verbosity.clone(),
+                )
+            };
+
+            #[cfg(not(all(target_os = "linux", feature = "safe-traversal")))]
+            let chown_result = wrap_chown(
                 path,
                 &meta,
                 self.dest_uid,
                 self.dest_gid,
                 self.dereference,
                 self.verbosity.clone(),
-            ) {
+            );
+
+            match chown_result {
                 Ok(n) => {
                     if !n.is_empty() {
                         show_error!("{n}");
@@ -351,6 +378,27 @@ impl ChownExecutor {
         }
     }
 
+    #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
+    fn safe_chown_dir(
+        &self,
+        dir_fd: &DirFd,
+        path: &Path,
+        meta: &Metadata,
+    ) -> Result<String, String> {
+        // Use fchown (safe) to change the directory's ownership
+        if let Err(e) = dir_fd.fchown(self.dest_uid, self.dest_gid) {
+            return Err(format!(
+                "cannot change ownership of '{}': {}",
+                path.display(),
+                strip_errno(&e)
+            ));
+        }
+
+        // Report the change if verbose (similar to wrap_chown)
+        self.report_ownership_change_success(path, meta.uid(), meta.gid());
+        Ok(String::new())
+    }
+
     #[cfg(all(target_os = "linux", feature = "safe-traversal"))]
     fn safe_dive_into<P: AsRef<Path>>(&self, root: P) -> i32 {
         let root = root.as_ref();