diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index f67db7c8c19..90070e962c6 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -317,6 +317,14 @@ struct Chmoder { mode_indices: Vec, } +#[cfg(not(target_os = "redox"))] +struct ChmodFrame { + dir_fd: DirFd, + dir_path: PathBuf, + dir_info: Option, + entries: std::vec::IntoIter, +} + impl Chmoder { /// Calculate the new mode based on the current mode and the chmod specification. /// Returns (`new_mode`, `naively_expected_new_mode`) for symbolic modes, or (`new_mode`, `new_mode`) for numeric/reference modes. @@ -652,7 +660,7 @@ impl Chmoder { // swapped-in symlink. `-H`/`-L` still follow, which is what they ask for. if (!file_path.is_symlink() || should_follow_symlink) && file_path.is_dir() { match DirFd::open(file_path, should_follow_symlink.into()) { - Ok(dir_fd) => self.safe_traverse_dir(&dir_fd, file_path, ancestors), + Ok(dir_fd) => self.safe_traverse_dir(dir_fd, file_path, ancestors), // A directory that does not open cannot be read. Err(err) => self.report_unreadable_dir(file_path, err), } @@ -662,7 +670,7 @@ impl Chmoder { #[cfg(not(target_os = "redox"))] fn safe_traverse_dir( &self, - dir_fd: &DirFd, + dir_fd: DirFd, dir_path: &Path, ancestors: &mut HashSet, ) { @@ -680,7 +688,7 @@ impl Chmoder { // Cycle detection: identify this directory by (dev, ino) via the already-open // fd. Using the fd is TOCTOU-safe (no path re-resolution through symlinks) and // avoids a redundant path walk. If it's already on the current path, it's a cycle. - let dir_info = FileInformation::from_file(dir_fd).ok(); + let dir_info = FileInformation::from_file(&dir_fd).ok(); if dir_info .as_ref() .is_some_and(|info| !ancestors.insert(info.clone())) @@ -691,67 +699,105 @@ impl Chmoder { // Determine if we should follow symlinks (doesn't depend on entry_name) let should_follow_symlink = self.traverse_symlinks == TraverseSymlinks::All; - for entry_name in entries { - let entry_path = dir_path.join(&entry_name); - - let meta = match dir_fd.metadata_at(&entry_name, should_follow_symlink.into()) { - Ok(meta) => meta, - Err(err) => { - // Handle permission denied with proper file path context - let error = if err.kind() == std::io::ErrorKind::PermissionDenied { - ChmodError::PermissionDenied(entry_path).into() - } else { - err.into() - }; - self.report_failure(error); - continue; - } - }; + let mut stack = vec![ChmodFrame { + dir_fd, + dir_path: dir_path.to_path_buf(), + dir_info, + entries: entries.into_iter(), + }]; + + while let Some(top) = stack.last_mut() { + if let Some(entry_name) = top.entries.next() { + let entry_path = top.dir_path.join(&entry_name); + + let meta = match top + .dir_fd + .metadata_at(&entry_name, should_follow_symlink.into()) + { + Ok(m) => m, + Err(e) => { + // Handle permission denied with proper file path context + let error = if e.kind() == std::io::ErrorKind::PermissionDenied { + ChmodError::PermissionDenied(entry_path).into() + } else { + e.into() + }; + self.report_failure(error); + continue; + } + }; - if entry_path.is_symlink() { - if let Err(err) = self.handle_symlink_during_safe_recursion( - &entry_path, - dir_fd, - &entry_name, - ancestors, - ) { - self.report_failure(err); - } - } else { - // 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. - if let Err(err) = self.safe_chmod_file( - &entry_path, - dir_fd, - &entry_name, - meta.mode() & 0o7777, - SymlinkBehavior::NoFollow, - ) { - self.report_failure(err); - } + if entry_path.is_symlink() { + if let Err(err) = self.handle_symlink_during_safe_recursion( + &entry_path, + &top.dir_fd, + &entry_name, + ancestors, + ) { + self.report_failure(err); + } + } else { + // 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. + if let Err(err) = self.safe_chmod_file( + &entry_path, + &top.dir_fd, + &entry_name, + meta.mode() & 0o7777, + SymlinkBehavior::NoFollow, + ) { + self.report_failure(err); + } - // 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, should_follow_symlink.into()) { - Ok(child_dir_fd) => { - self.safe_traverse_dir(&child_dir_fd, &entry_path, ancestors); + // Descend into subdirectories iteratively 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 top + .dir_fd + .open_subdir(&entry_name, should_follow_symlink.into()) + { + Ok(child_dir_fd) => { + let child_info = FileInformation::from_file(&child_dir_fd).ok(); + if child_info + .as_ref() + .is_some_and(|info| !ancestors.insert(info.clone())) + { + continue; // cycle detected: skip entering child + } + + match child_dir_fd.read_dir() { + Ok(child_entries) => { + stack.push(ChmodFrame { + dir_fd: child_dir_fd, + dir_path: entry_path, + dir_info: child_info, + entries: child_entries.into_iter(), + }); + } + Err(err) => { + if let Some(info) = child_info { + ancestors.remove(&info); + } + self.report_unreadable_dir(&entry_path, err); + } + } + } + Err(err) => { + self.report_unreadable_dir(&entry_path, err); + } } - Err(err) => self.report_unreadable_dir(&entry_path, err), } } + } else if let Some(info) = stack.pop().and_then(|f| f.dir_info) { + // Backtrack so sibling subtrees that legitimately reach the same directory + // (e.g. two symlinks to one dir) are not mistaken for cycles. + ancestors.remove(&info); } } - - // Backtrack so sibling subtrees that legitimately reach the same directory - // (e.g. two symlinks to one dir) are not mistaken for cycles. - if let Some(info) = dir_info { - ancestors.remove(&info); - } } #[cfg(not(target_os = "redox"))] diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index cd95da9fa2f..ee8c634223b 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -212,6 +212,18 @@ impl Stat { latest_time, }) } + + #[cfg(all(unix, not(target_os = "redox")))] + fn merge(&mut self, other: &Self) { + self.size += other.size; + self.blocks += other.blocks; + self.inodes += other.inodes; + self.latest_time = match (self.latest_time, other.latest_time) { + (Some(a), Some(b)) => Some(a.max(b)), + (a, None) => a, + (None, b) => b, + }; + } } #[cfg(not(windows))] @@ -346,56 +358,67 @@ fn time_from_raw_stat( } } +#[cfg(all(unix, not(target_os = "redox")))] +const S_IFMT: u32 = 0o170_000; +#[cfg(all(unix, not(target_os = "redox")))] +const S_IFDIR: u32 = 0o040_000; +#[cfg(all(unix, not(target_os = "redox")))] +const S_IFLNK: u32 = 0o120_000; + +#[cfg(all(unix, not(target_os = "redox")))] +struct DuFrame { + dir_fd: DirFd, + depth: usize, + my_stat: Stat, + entries: std::vec::IntoIter, +} + #[cfg(all(unix, not(target_os = "redox")))] // Implement safe_du on Unix (except Redox which lacks full stat support) -// This is done for TOCTOU safety +// Iterative implementation using a heap-allocated stack to prevent stack overflow on deep directory trees (#10173). fn safe_du( path: &Path, options: &TraversalOptions, - depth: usize, seen_inodes: &mut FxHashSet, print_tx: &mpsc::Sender>, - parent_fd: Option<&DirFd>, - initial_stat: Option>, + initial_stat: io::Result, ) -> Result>>> { - // The caller provides an already-computed stat for this entry, which lets us - // avoid re-stating it here. For subdirectories this saves both a redundant - // `fstatat` and an expensive full-path `lstat` per directory (the dominant cost - // when traversing deep trees). The root directory may instead carry an error, - // in which case we fall back to opening it directly with a DirFd. - let initial_stat = initial_stat.unwrap_or_else(|| Stat::new(path, None, options)); - let mut my_stat = match initial_stat { - Ok(mut s) => { - // For subdirectories the `metadata` field is a cheap placeholder (the - // parent directory's metadata). It is only consulted by `--time`, so we - // only pay for a real stat of this entry when time reporting is asked for. - if options.time.is_some() - && parent_fd.is_some() - && let Ok(md) = fs::symlink_metadata(&s.path) - { - s.metadata = md; + // The caller provides an already-computed stat for this root entry, which lets us + // avoid re-stating it here. If the stat failed, fall back to opening directly with DirFd. + let (my_stat, dir_fd) = match initial_stat { + Ok(s) => { + if !s.metadata.is_dir() { + return Ok(s); + } + match DirFd::open(path, SymlinkBehavior::Follow) { + Ok(fd) => (s, fd), + Err(e) => { + print_tx.send(Err(e.map_err_context( + || translate!("du-error-cannot-read-directory", "path" => path.quote()), + )))?; + return Ok(s); + } } - s } Err(_e) => { - // Only the root path can reach this branch (subdirectories always pass a - // valid stat). Try using our DirFd method for the root directory. - match DirFd::open(path, SymlinkBehavior::Follow) { - Ok(dir_fd) => match Stat::new_from_dirfd(&dir_fd, path, options.time) { - Ok(s) => s, - Err(e) => { - let error = e.map_err_context( - || translate!("du-error-cannot-access", "path" => path.quote()), - ); - if let Err(send_error) = print_tx.send(Err(error)) { - return Err(Box::new(send_error)); - } - return Err(Box::new(mpsc::SendError(Err(USimpleError::new( - 0, - "Error already handled", - ))))); + // Only the root path can reach this branch. Try using our DirFd method for the root directory. + let dir_fd = match DirFd::open(path, SymlinkBehavior::Follow) { + Ok(dir_fd) => dir_fd, + Err(e) => { + let error = e.map_err_context( + || translate!("du-error-cannot-access", "path" => path.quote()), + ); + if let Err(send_error) = print_tx.send(Err(error)) { + return Err(Box::new(send_error)); } - }, + return Err(Box::new(mpsc::SendError(Err(USimpleError::new( + 0, + "Error already handled", + ))))); + } + }; + let s = match Stat::new_from_dirfd(&dir_fd, path, options.time) { + Ok(s) => s, Err(e) => { let error = e.map_err_context( || translate!("du-error-cannot-access", "path" => path.quote()), @@ -408,29 +431,11 @@ fn safe_du( "Error already handled", ))))); } + }; + if !s.metadata.is_dir() { + return Ok(s); } - } - }; - if !my_stat.metadata.is_dir() { - return Ok(my_stat); - } - - // Open the directory using DirFd - let open_result = match parent_fd { - Some(parent) => parent.open_subdir( - path.file_name().unwrap_or(path.as_os_str()), - SymlinkBehavior::Follow, - ), - None => DirFd::open(path, SymlinkBehavior::Follow), - }; - - let dir_fd = match open_result { - Ok(fd) => fd, - Err(e) => { - print_tx.send(Err(e.map_err_context( - || translate!("du-error-cannot-read-directory", "path" => path.quote()), - )))?; - return Ok(my_stat); + (s, dir_fd) } }; @@ -445,160 +450,197 @@ fn safe_du( } }; - 'file_loop: for entry_name in entries { - const S_IFMT: u32 = 0o170_000; - const S_IFDIR: u32 = 0o040_000; - const S_IFLNK: u32 = 0o120_000; + let mut stack = vec![DuFrame { + dir_fd, + depth: 0, + my_stat, + entries: entries.into_iter(), + }]; + + while let Some(top) = stack.last_mut() { + if let Some(entry_name) = top.entries.next() { + // First get the lstat (without following symlinks) to check if it's a symlink + let lstat = match top.dir_fd.stat_at(&entry_name, SymlinkBehavior::NoFollow) { + Ok(stat) => stat, + Err(e) => { + let entry_path = top.my_stat.path.join(&entry_name); + print_tx.send(Err(e.map_err_context( + || translate!("du-error-cannot-access", "path" => entry_path.quote()), + )))?; + continue; + } + }; - // First get the lstat (without following symlinks) to check if it's a symlink - let lstat = match dir_fd.stat_at(&entry_name, SymlinkBehavior::NoFollow) { - Ok(stat) => stat, - Err(e) => { - let entry_path = path.join(&entry_name); - print_tx.send(Err(e.map_err_context( - || translate!("du-error-cannot-access", "path" => entry_path.quote()), - )))?; + // Check if it's a symlink + #[allow(clippy::unnecessary_cast)] + let is_symlink = (lstat.st_mode as u32 & S_IFMT) == S_IFLNK; + + // Handle symlinks with -L option + // For safe traversal with -L, we skip symlinks to directories entirely + // and let the non-safe traversal handle them at the top level + if is_symlink && options.dereference == Deref::All { + // Skip symlinks to directories when using safe traversal with -L + // They will be handled by regular traversal continue; } - }; - // Check if it's a symlink - #[allow(clippy::unnecessary_cast)] - let is_symlink = (lstat.st_mode as u32 & S_IFMT) == S_IFLNK; - - // Handle symlinks with -L option - // For safe traversal with -L, we skip symlinks to directories entirely - // and let the non-safe traversal handle them at the top level - if is_symlink && options.dereference == Deref::All { - // Skip symlinks to directories when using safe traversal with -L - // They will be handled by regular traversal - continue; - } - - #[allow(clippy::unnecessary_cast)] - let is_dir = (lstat.st_mode as u32 & S_IFMT) == S_IFDIR; - let entry_stat = lstat; + #[allow(clippy::unnecessary_cast)] + let is_dir = (lstat.st_mode as u32 & S_IFMT) == S_IFDIR; + let entry_stat = lstat; - #[allow(clippy::unnecessary_cast)] - let file_info = (entry_stat.st_ino != 0).then_some(FileInfo { - file_id: entry_stat.st_ino as u128, - dev_id: entry_stat.st_dev as u64, - }); + #[allow(clippy::unnecessary_cast)] + let file_info = (entry_stat.st_ino != 0).then_some(FileInfo { + file_id: entry_stat.st_ino as u128, + dev_id: entry_stat.st_dev as u64, + }); - let safe_metadata = uucore::safe_traversal::Metadata::from_stat(entry_stat); - let latest_time = options - .time - .and_then(|time| time_from_raw_stat(&safe_metadata, time)); - - // For safe traversal, we need to handle stats differently - // We can't use std::fs::Metadata since that requires the full path - let this_stat = if is_dir { - // For directories, recurse using safe_du - Stat { - path: path.join(&entry_name), - size: 0, - #[allow(clippy::unnecessary_cast)] - blocks: entry_stat.st_blocks as u64, - inodes: 1, - inode: file_info, - // We need a fake metadata - create one from symlink_metadata of parent - // This is a workaround since we can't get real metadata without the full path - metadata: my_stat.metadata.clone(), - latest_time, - } - } else { - // For files - Stat { - path: path.join(&entry_name), - #[allow(clippy::unnecessary_cast)] - size: entry_stat.st_size as u64, - #[allow(clippy::unnecessary_cast)] - blocks: entry_stat.st_blocks as u64, - inodes: 1, - inode: file_info, - metadata: my_stat.metadata.clone(), - latest_time, - } - }; + let safe_metadata = uucore::safe_traversal::Metadata::from_stat(entry_stat); + let latest_time = options + .time + .and_then(|time| time_from_raw_stat(&safe_metadata, time)); + + let child_path = top.my_stat.path.join(&entry_name); + + // For safe traversal, we need to handle stats differently + // We can't use std::fs::Metadata since that requires the full path + let this_stat = if is_dir { + let mut s = Stat { + path: child_path.clone(), + size: 0, + #[allow(clippy::unnecessary_cast)] + blocks: entry_stat.st_blocks as u64, + inodes: 1, + inode: file_info, + // We need a fake metadata - create one from symlink_metadata of parent + // This is a workaround since we can't get real metadata without the full path + metadata: top.my_stat.metadata.clone(), + latest_time, + }; + if options.time.is_some() + && let Ok(md) = fs::symlink_metadata(&s.path) + { + s.metadata = md; + } + s + } else { + // For files + Stat { + path: child_path, + #[allow(clippy::unnecessary_cast)] + size: entry_stat.st_size as u64, + #[allow(clippy::unnecessary_cast)] + blocks: entry_stat.st_blocks as u64, + inodes: 1, + inode: file_info, + metadata: top.my_stat.metadata.clone(), + latest_time, + } + }; - // Check excludes - for pattern in &options.excludes { - if pattern.matches(&this_stat.path.to_string_lossy()) - || pattern.matches(&entry_name.to_string_lossy()) - { - if options.verbose { - println!( - "{}", - translate!("du-verbose-ignored", "path" => this_stat.path.quote()) - ); + // Check excludes + let mut ignored = false; + for pattern in &options.excludes { + if pattern.matches(&this_stat.path.to_string_lossy()) + || pattern.matches(&entry_name.to_string_lossy()) + { + if options.verbose { + println!( + "{}", + translate!("du-verbose-ignored", "path" => this_stat.path.quote()) + ); + } + ignored = true; + break; } - continue 'file_loop; } - } - - // Handle inodes - if let Some(inode) = this_stat.inode - && !seen_inodes.insert(inode) - && !options.count_links - { - continue; - } + if ignored { + continue; + } - // Process directories recursively - if is_dir { - if options.one_file_system - && let (Some(this_inode), Some(my_inode)) = (this_stat.inode, my_stat.inode) - && this_inode.dev_id != my_inode.dev_id + // Handle inodes + if let Some(inode) = this_stat.inode + && !seen_inodes.insert(inode) + && !options.count_links { continue; } - // Reuse the stat we already computed for this entry instead of - // re-stating it inside the recursive call. - let sub_path = this_stat.path.clone(); - let this_stat = safe_du( - &sub_path, - options, - depth + 1, - seen_inodes, - print_tx, - Some(&dir_fd), - Some(Ok(this_stat)), - )?; + // Process directories iteratively + if is_dir { + if options.one_file_system + && let (Some(this_inode), Some(my_inode)) = (this_stat.inode, top.my_stat.inode) + && this_inode.dev_id != my_inode.dev_id + { + continue; + } - if !options.separate_dirs { - my_stat.size += this_stat.size; - my_stat.blocks += this_stat.blocks; - my_stat.inodes += this_stat.inodes; - my_stat.latest_time = match (my_stat.latest_time, this_stat.latest_time) { - (Some(a), Some(b)) => Some(a.max(b)), - (a, None) => a, - (None, b) => b, + let child_depth = top.depth + 1; + let open_subdir_res = top.dir_fd.open_subdir(&entry_name, SymlinkBehavior::Follow); + let child_dir_fd = match open_subdir_res { + Ok(fd) => fd, + Err(e) => { + print_tx.send(Err(e.map_err_context( + || translate!("du-error-cannot-read-directory", "path" => this_stat.path.quote()), + )))?; + if !options.separate_dirs { + top.my_stat.merge(&this_stat); + } + print_tx.send(Ok(StatPrintInfo { + stat: this_stat, + depth: child_depth, + }))?; + continue; + } + }; + + let child_entries = match child_dir_fd.read_dir() { + Ok(entries) => entries, + Err(e) => { + print_tx.send(Err(e.map_err_context( + || translate!("du-error-cannot-read-directory", "path" => this_stat.path.quote()), + )))?; + if !options.separate_dirs { + top.my_stat.merge(&this_stat); + } + print_tx.send(Ok(StatPrintInfo { + stat: this_stat, + depth: child_depth, + }))?; + continue; + } + }; + + stack.push(DuFrame { + dir_fd: child_dir_fd, + depth: child_depth, + my_stat: this_stat, + entries: child_entries.into_iter(), + }); + } else { + top.my_stat.merge(&this_stat); + if options.all { + print_tx.send(Ok(StatPrintInfo { + stat: this_stat, + depth: top.depth + 1, + }))?; } } - print_tx.send(Ok(StatPrintInfo { - stat: this_stat, - depth: depth + 1, - }))?; - } else { - my_stat.size += this_stat.size; - my_stat.blocks += this_stat.blocks; - my_stat.inodes += 1; - my_stat.latest_time = match (my_stat.latest_time, this_stat.latest_time) { - (Some(a), Some(b)) => Some(a.max(b)), - (a, None) => a, - (None, b) => b, - }; - if options.all { + } else if let Some(finished) = stack.pop() { + if let Some(parent) = stack.last_mut() { + if !options.separate_dirs { + parent.my_stat.merge(&finished.my_stat); + } print_tx.send(Ok(StatPrintInfo { - stat: this_stat, - depth: depth + 1, + stat: finished.my_stat, + depth: finished.depth, }))?; + } else { + return Ok(finished.my_stat); } } } - Ok(my_stat) + unreachable!("iterative du stack drained without returning root stat") } // this takes `my_stat` to avoid having to stat files multiple times. @@ -1208,15 +1250,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Use safe traversal (Unix except Redox, when not using -L) #[cfg(all(unix, not(target_os = "redox")))] { - match safe_du( - &path, - &traversal_options, - 0, - &mut seen_inodes, - &print_tx, - None, - Some(stat), - ) { + match safe_du(&path, &traversal_options, &mut seen_inodes, &print_tx, stat) { Ok(stat) => { print_tx .send(Ok(StatPrintInfo { stat, depth: 0 })) diff --git a/src/uucore/locales/en-US.ftl b/src/uucore/locales/en-US.ftl index 6afc1ab7647..663b4ce97d5 100644 --- a/src/uucore/locales/en-US.ftl +++ b/src/uucore/locales/en-US.ftl @@ -100,6 +100,7 @@ mode-diag-help-syntax = a mode is either octal, as in 644, or clauses such as u+ # Shared recursive chown/chgrp/chmod diagnostics (uucore::perms) perms-cannot-access = cannot access { $file }: { $error } perms-cannot-access-replaced = cannot access { $file }: replaced while it was being processed +perms-cannot-read-directory = cannot read directory { $file }: { $error } # Encoding/decoding error messages (base32, base64, basenc) encoding-error-invalid-input = invalid input diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index c056c3fa05e..3043d2b270e 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -275,6 +275,14 @@ pub fn get_metadata(file: &Path, follow: bool) -> std::io::Result { } } +#[cfg(target_os = "linux")] +struct PermsFrame { + dir_fd: DirFd, + dir_path: std::path::PathBuf, + dir_info: Option, + entries: std::vec::IntoIter, +} + impl ChownExecutor { pub fn exec(&self) -> UResult<()> { use std::io::Write; @@ -511,14 +519,14 @@ impl ChownExecutor { let mut ancestors = HashSet::new(); let mut ret = 0; - self.safe_traverse_dir(&dir_fd, root, &mut ret, &mut ancestors); + self.safe_traverse_dir(dir_fd, root, &mut ret, &mut ancestors); ret } #[cfg(target_os = "linux")] fn safe_traverse_dir( &self, - dir_fd: &DirFd, + dir_fd: DirFd, dir_path: &Path, ret: &mut i32, ancestors: &mut HashSet, @@ -526,7 +534,7 @@ impl ChownExecutor { // Cycle detection: identify this directory by (dev, ino) via the already-open // fd. Using the fd is TOCTOU-safe (no path re-resolution through symlinks) and // avoids a redundant path walk. If it's already on the current path, it's a cycle. - let dir_info = FileInformation::from_file(dir_fd).ok(); + let dir_info = FileInformation::from_file(&dir_fd).ok(); if dir_info .as_ref() .is_some_and(|info| !ancestors.insert(info.clone())) @@ -541,109 +549,159 @@ impl ChownExecutor { *ret = 1; if self.verbosity.level != VerbosityLevel::Silent { show_error!( - "cannot read directory {}: {}", - dir_path.quote(), - strip_errno(&e) + "{}", + translate!( + "perms-cannot-read-directory", + "file" => dir_path.quote(), + "error" => strip_errno(&e) + ) ); } + if let Some(info) = dir_info { + ancestors.remove(&info); + } return; } }; - for entry_name in entries { - let entry_path = dir_path.join(&entry_name); + let mut stack = vec![PermsFrame { + dir_fd, + dir_path: dir_path.to_path_buf(), + dir_info, + entries: entries.into_iter(), + }]; - // Get metadata for the entry - let follow = self.traverse_symlinks == TraverseSymlinks::All; + while let Some(top) = stack.last_mut() { + if let Some(entry_name) = top.entries.next() { + let entry_path = top.dir_path.join(&entry_name); - let meta = match dir_fd.metadata_at(&entry_name, follow.into()) { - Ok(m) => m, - Err(e) => { - *ret = 1; - if self.verbosity.level != VerbosityLevel::Silent { - show_error!( - "{}", - translate!("perms-cannot-access", "file" => entry_path.quote(), "error" => strip_errno(&e)) - ); - } - continue; - } - }; + // Get metadata for the entry + let follow = self.traverse_symlinks == TraverseSymlinks::All; - if self.preserve_root - && is_root(&entry_path, self.traverse_symlinks == TraverseSymlinks::All) - { - *ret = 1; - return; - } - - // Check if we should chown this entry - if self.matched(meta.uid(), meta.gid()) { - // Use fchownat for the actual ownership change - let follow_symlinks = - self.dereference || self.traverse_symlinks == TraverseSymlinks::All; - - // Only pass the IDs that should actually be changed - let chown_uid = self.dest_uid; - let chown_gid = self.dest_gid; + let meta = match top.dir_fd.metadata_at(&entry_name, follow.into()) { + Ok(meta) => meta, + Err(e) => { + *ret = 1; + if self.verbosity.level != VerbosityLevel::Silent { + show_error!( + "{}", + translate!("perms-cannot-access", "file" => entry_path.quote(), "error" => strip_errno(&e)) + ); + } + continue; + } + }; - if let Err(e) = - dir_fd.chown_at(&entry_name, chown_uid, chown_gid, follow_symlinks.into()) + if self.preserve_root + && is_root(&entry_path, self.traverse_symlinks == TraverseSymlinks::All) { *ret = 1; - if self.verbosity.level != VerbosityLevel::Silent { - let msg = format!( - "changing {} of {}: {}", - if self.verbosity.groups_only { - "group" - } else { - "ownership" - }, - entry_path.quote(), - strip_errno(&e) - ); - show_error!("{msg}"); + if let Some(info) = stack.pop().and_then(|f| f.dir_info) { + ancestors.remove(&info); } - } else { - // Report the successful ownership change using the shared helper - self.report_ownership_change_success(&entry_path, meta.uid(), meta.gid()); + continue; } - } else if self.print_verbose_ownership_retained_as( - &entry_path, - meta.uid(), - self.dest_gid.map(|_| meta.gid()), - ) != 0 - { - *ret = 1; - } - // 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, follow.into()) { - Ok(subdir_fd) => { - self.safe_traverse_dir(&subdir_fd, &entry_path, ret, ancestors); - } - Err(e) => { + // Check if we should chown this entry + if self.matched(meta.uid(), meta.gid()) { + // Use fchownat for the actual ownership change + let follow_symlinks = + self.dereference || self.traverse_symlinks == TraverseSymlinks::All; + + // Only pass the IDs that should actually be changed + let chown_uid = self.dest_uid; + let chown_gid = self.dest_gid; + + if let Err(e) = top.dir_fd.chown_at( + &entry_name, + chown_uid, + chown_gid, + follow_symlinks.into(), + ) { *ret = 1; if self.verbosity.level != VerbosityLevel::Silent { - show_error!( - "{}", - translate!("perms-cannot-access", "file" => entry_path.quote(), "error" => strip_errno(&e)) + let msg = format!( + "changing {} of {}: {}", + if self.verbosity.groups_only { + "group" + } else { + "ownership" + }, + entry_path.quote(), + strip_errno(&e) ); + show_error!("{msg}"); } + } else { + // Report the successful ownership change using the shared helper + self.report_ownership_change_success(&entry_path, meta.uid(), meta.gid()); } + } else if self.print_verbose_ownership_retained_as( + &entry_path, + meta.uid(), + self.dest_gid.map(|_| meta.gid()), + ) != 0 + { + *ret = 1; } - } - } - // Backtrack so sibling subtrees that legitimately reach the same directory - // (e.g. two symlinks to one dir) are not mistaken for cycles. - if let Some(info) = dir_info { - ancestors.remove(&info); + // Descend into subdirectories iteratively. 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 top.dir_fd.open_subdir(&entry_name, follow.into()) { + Ok(subdir_fd) => { + let child_info = FileInformation::from_file(&subdir_fd).ok(); + if child_info + .as_ref() + .is_some_and(|info| !ancestors.insert(info.clone())) + { + continue; // cycle detected, skip + } + + match subdir_fd.read_dir() { + Ok(child_entries) => { + stack.push(PermsFrame { + dir_fd: subdir_fd, + dir_path: entry_path, + dir_info: child_info, + entries: child_entries.into_iter(), + }); + } + Err(e) => { + if let Some(info) = child_info { + ancestors.remove(&info); + } + *ret = 1; + if self.verbosity.level != VerbosityLevel::Silent { + show_error!( + "{}", + translate!( + "perms-cannot-read-directory", + "file" => entry_path.quote(), + "error" => strip_errno(&e) + ) + ); + } + } + } + } + Err(e) => { + *ret = 1; + if self.verbosity.level != VerbosityLevel::Silent { + show_error!( + "{}", + translate!("perms-cannot-access", "file" => entry_path.quote(), "error" => strip_errno(&e)) + ); + } + } + } + } + } else if let Some(info) = stack.pop().and_then(|f| f.dir_info) { + ancestors.remove(&info); + } } } diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 0fe8b5192de..cd9b8f67370 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -2029,3 +2029,24 @@ fn test_overlong_path_reports_os_error_and_continues() { "chmod: failed to get attributes of '{path}': {error}\n" )); } + +#[cfg(target_os = "linux")] +#[test] +fn test_deep_directory_traversal_no_stack_overflow() { + use rlimit::Resource; + use uutests::util::prepare_deep_traversal; + + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("root_dir"); + let root = at.plus("root_dir"); + let Some(file_limit) = prepare_deep_traversal(&root) else { + return; + }; + + ucmd.limit(Resource::NOFILE, file_limit, file_limit) + .limit(Resource::STACK, 256 * 1024, 256 * 1024) + .arg("-R") + .arg("755") + .arg("root_dir") + .succeeds(); +} diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 3f561370841..3ab1c695843 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -1168,3 +1168,25 @@ fn test_repeated_flags_are_accepted() { .succeeds(); } } + +#[cfg(target_os = "linux")] +#[test] +fn test_deep_directory_traversal_no_stack_overflow() { + use rlimit::Resource; + use rustix::process::geteuid; + use uutests::util::prepare_deep_traversal; + + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("root_dir"); + let root = at.plus("root_dir"); + let Some(file_limit) = prepare_deep_traversal(&root) else { + return; + }; + + ucmd.limit(Resource::NOFILE, file_limit, file_limit) + .limit(Resource::STACK, 256 * 1024, 256 * 1024) + .arg("-R") + .arg(geteuid().to_string()) + .arg("root_dir") + .succeeds(); +} diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 39cc4626997..4c2e0cc6907 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -2938,3 +2938,22 @@ du: invalid suffix in --block-size argument '1fb' .stderr_is("du: invalid suffix in --block-size argument '1fb'\n"); } } + +#[test] +#[cfg(target_os = "linux")] +fn test_deep_directory_traversal_no_stack_overflow() { + use rlimit::Resource; + use uutests::util::prepare_deep_traversal; + + let ts = TestScenario::new(util_name!()); + let Some(file_limit) = prepare_deep_traversal(&ts.fixtures.subdir) else { + return; + }; + + ts.ucmd() + .limit(Resource::NOFILE, file_limit, file_limit) + .limit(Resource::STACK, 256 * 1024, 256 * 1024) + .arg("-s") + .arg(".") + .succeeds(); +} diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 25db32151de..cf4e4a474e0 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -25,7 +25,7 @@ use nix::sys; use nix::sys::stat::{self, SFlag}; use pretty_assertions::assert_eq; #[cfg(unix)] -use rlimit::{Resource, setrlimit}; +use rlimit::{Resource, getrlimit, setrlimit}; use std::borrow::Cow; use std::collections::{BTreeMap, VecDeque}; use std::ffi::{OsStr, OsString}; @@ -3400,6 +3400,39 @@ pub fn run_ucmd_as_root_with_stdin_stdout( } } +/// Prepares a deep directory tree for stack overflow regression testing. +/// +/// Returns `Some(file_limit)` if the system's hard `RLIMIT_NOFILE` limit is at least 2000 +/// and directory creation succeeded, or `None` if the test should be skipped (e.g. in environments +/// where `NOFILE` cannot be raised enough to avoid `EMFILE`). +#[cfg(unix)] +#[must_use] +pub fn prepare_deep_traversal(root: &Path) -> Option { + let (_, hard) = getrlimit(Resource::NOFILE).ok()?; + let max_depth = if cfg!(target_os = "linux") { 1500 } else { 400 }; + let required_fd = (max_depth as u64) + 50; + + let (file_limit, target_depth) = if hard >= required_fd { + (hard.min(4096), max_depth) + } else if hard >= 600 { + let depth = (hard.saturating_sub(50)) as usize; + (hard, depth) + } else { + println!("skipping deep traversal test: hard NOFILE limit {hard} < 600"); + return None; + }; + + let mut current = root.to_path_buf(); + for _ in 0..target_depth { + current.push("d"); + if fs::create_dir(¤t).is_err() { + return None; + } + } + + Some(file_limit) +} + /// Sanity checks for test utils #[cfg(test)] mod tests {