Skip to content

Commit 30239e6

Browse files
feat: Expand safe directory traversal to all Unix platforms and fix related type conversions. (uutils#9792)
--------- Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
1 parent 2b1d40a commit 30239e6

File tree

17 files changed

+145
-130
lines changed

17 files changed

+145
-130
lines changed

.vscode/cspell.dictionaries/acronyms+names.wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ RISCV
3434
RNG # random number generator
3535
RNGs
3636
Solaris
37+
TOCTOU # time-of-check time-of-use
3738
UID # user ID
3839
UIDs
3940
UUID # universally unique identifier

src/uu/chmod/Cargo.toml

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,12 @@ path = "src/chmod.rs"
2020
[dependencies]
2121
clap = { workspace = true }
2222
thiserror = { workspace = true }
23-
uucore = { workspace = true, features = [
24-
"entries",
25-
"fs",
26-
"mode",
27-
"perms",
28-
"safe-traversal",
29-
] }
23+
uucore = { workspace = true, features = ["entries", "fs", "mode", "perms"] }
3024
fluent = { workspace = true }
3125

26+
[target.'cfg(all(unix, not(target_os = "redox")))'.dependencies]
27+
uucore = { workspace = true, features = ["safe-traversal"] }
28+
3229
[[bin]]
3330
name = "chmod"
3431
path = "src/main.rs"

src/uu/chmod/src/chmod.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use uucore::libc::mode_t;
1818
use uucore::mode;
1919
use uucore::perms::{TraverseSymlinks, configure_symlink_and_recursion};
2020

21-
#[cfg(target_os = "linux")]
21+
#[cfg(all(unix, not(target_os = "redox")))]
2222
use uucore::safe_traversal::DirFd;
2323
use uucore::{format_usage, show, show_error};
2424

@@ -338,7 +338,7 @@ impl Chmoder {
338338
}
339339

340340
/// Handle symlinks during directory traversal based on traversal mode
341-
#[cfg(not(target_os = "linux"))]
341+
#[cfg(not(unix))]
342342
fn handle_symlink_during_traversal(
343343
&self,
344344
path: &Path,
@@ -423,7 +423,8 @@ impl Chmoder {
423423
matches!(fs::canonicalize(&file), Ok(p) if p == Path::new("/"))
424424
}
425425

426-
#[cfg(not(target_os = "linux"))]
426+
// Non-safe traversal implementation for platforms without safe_traversal support
427+
#[cfg(any(not(unix), target_os = "redox"))]
427428
fn walk_dir_with_context(&self, file_path: &Path, is_command_line_arg: bool) -> UResult<()> {
428429
let mut r = self.chmod_file(file_path);
429430

@@ -436,8 +437,7 @@ impl Chmoder {
436437

437438
// If the path is a directory (or we should follow symlinks), recurse into it
438439
if (!file_path.is_symlink() || should_follow_symlink) && file_path.is_dir() {
439-
// We buffer all paths in this dir to not keep to be able to close the fd so not
440-
// too many fd's are open during the recursion
440+
// We buffer all paths in this dir to not keep too many fd's open during recursion
441441
let mut paths_in_this_dir = Vec::new();
442442

443443
for dir_entry in file_path.read_dir()? {
@@ -450,17 +450,24 @@ impl Chmoder {
450450
}
451451
}
452452
for path in paths_in_this_dir {
453-
if path.is_symlink() {
454-
r = self.handle_symlink_during_recursion(&path).and(r);
455-
} else {
453+
#[cfg(not(unix))]
454+
{
455+
if path.is_symlink() {
456+
r = self.handle_symlink_during_recursion(&path).and(r);
457+
} else {
458+
r = self.walk_dir_with_context(path.as_path(), false).and(r);
459+
}
460+
}
461+
#[cfg(target_os = "redox")]
462+
{
456463
r = self.walk_dir_with_context(path.as_path(), false).and(r);
457464
}
458465
}
459466
}
460467
r
461468
}
462469

463-
#[cfg(target_os = "linux")]
470+
#[cfg(all(unix, not(target_os = "redox")))]
464471
fn walk_dir_with_context(&self, file_path: &Path, is_command_line_arg: bool) -> UResult<()> {
465472
let mut r = self.chmod_file(file_path);
466473

@@ -490,7 +497,7 @@ impl Chmoder {
490497
r
491498
}
492499

493-
#[cfg(target_os = "linux")]
500+
#[cfg(all(unix, not(target_os = "redox")))]
494501
fn safe_traverse_dir(&self, dir_fd: &DirFd, dir_path: &Path) -> UResult<()> {
495502
let mut r = Ok(());
496503

@@ -546,7 +553,7 @@ impl Chmoder {
546553
r
547554
}
548555

549-
#[cfg(target_os = "linux")]
556+
#[cfg(all(unix, not(target_os = "redox")))]
550557
fn handle_symlink_during_safe_recursion(
551558
&self,
552559
path: &Path,
@@ -578,7 +585,7 @@ impl Chmoder {
578585
}
579586
}
580587

581-
#[cfg(target_os = "linux")]
588+
#[cfg(all(unix, not(target_os = "redox")))]
582589
fn safe_chmod_file(
583590
&self,
584591
file_path: &Path,
@@ -608,7 +615,7 @@ impl Chmoder {
608615
Ok(())
609616
}
610617

611-
#[cfg(not(target_os = "linux"))]
618+
#[cfg(not(unix))]
612619
fn handle_symlink_during_recursion(&self, path: &Path) -> UResult<()> {
613620
// Use the common symlink handling logic
614621
self.handle_symlink_during_traversal(path, false)

src/uu/cp/src/cp.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,6 +1566,7 @@ fn file_mode_for_interactive_overwrite(
15661566
match path.metadata() {
15671567
Ok(me) => {
15681568
// Cast is necessary on some platforms
1569+
#[allow(clippy::unnecessary_cast)]
15691570
let mode: mode_t = me.mode() as mode_t;
15701571

15711572
// It looks like this extra information is added to the prompt iff the file's user write bit is 0
@@ -1758,7 +1759,7 @@ pub(crate) fn copy_attributes(
17581759
Ok(())
17591760
})?;
17601761

1761-
#[cfg(feature = "selinux")]
1762+
#[cfg(all(feature = "selinux", target_os = "linux"))]
17621763
handle_preserve(&attributes.context, || -> CopyResult<()> {
17631764
// Get the source context and apply it to the destination
17641765
if let Ok(context) = selinux::SecurityContext::of_path(source, false, false) {
@@ -2552,7 +2553,7 @@ fn copy_file(
25522553
copy_attributes(source, dest, &options.attributes)?;
25532554
}
25542555

2555-
#[cfg(feature = "selinux")]
2556+
#[cfg(all(feature = "selinux", target_os = "linux"))]
25562557
if options.set_selinux_context && uucore::selinux::is_selinux_enabled() {
25572558
// Set the given selinux permissions on the copied file.
25582559
if let Err(e) =
@@ -2620,8 +2621,10 @@ fn handle_no_preserve_mode(options: &Options, org_mode: u32) -> u32 {
26202621
target_os = "redox",
26212622
))]
26222623
{
2624+
#[allow(clippy::unnecessary_cast)]
26232625
const MODE_RW_UGO: u32 =
26242626
(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) as u32;
2627+
#[allow(clippy::unnecessary_cast)]
26252628
const S_IRWXUGO: u32 = (S_IRWXU | S_IRWXG | S_IRWXO) as u32;
26262629
return if is_explicit_no_preserve_mode {
26272630
MODE_RW_UGO

src/uu/du/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ uucore = { workspace = true, features = [
2727
"parser-size",
2828
"parser-glob",
2929
"time",
30-
"safe-traversal",
3130
] }
3231
thiserror = { workspace = true }
3332
fluent = { workspace = true }
3433

34+
[target.'cfg(all(unix, not(target_os = "redox")))'.dependencies]
35+
uucore = { workspace = true, features = ["safe-traversal"] }
36+
3537
[target.'cfg(target_os = "windows")'.dependencies]
3638
windows-sys = { workspace = true, features = [
3739
"Win32_Storage_FileSystem",

src/uu/du/src/du.rs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use uucore::display::{Quotable, print_verbatim};
2525
use uucore::error::{FromIo, UError, UResult, USimpleError, set_exit_code};
2626
use uucore::fsext::{MetadataTimeField, metadata_get_time};
2727
use uucore::line_ending::LineEnding;
28-
#[cfg(target_os = "linux")]
28+
#[cfg(all(unix, not(target_os = "redox")))]
2929
use uucore::safe_traversal::DirFd;
3030
use uucore::translate;
3131

@@ -164,7 +164,7 @@ impl Stat {
164164
}
165165

166166
/// Create a Stat using safe traversal methods with `DirFd` for the root directory
167-
#[cfg(target_os = "linux")]
167+
#[cfg(all(unix, not(target_os = "redox")))]
168168
fn new_from_dirfd(dir_fd: &DirFd, full_path: &Path) -> std::io::Result<Self> {
169169
// Get metadata for the directory itself using fstat
170170
let safe_metadata = dir_fd.metadata()?;
@@ -293,9 +293,9 @@ fn read_block_size(s: Option<&str>) -> UResult<u64> {
293293
}
294294
}
295295

296-
#[cfg(target_os = "linux")]
297-
// For now, implement safe_du only on Linux
298-
// This is done for Ubuntu but should be extended to other platforms that support openat
296+
#[cfg(all(unix, not(target_os = "redox")))]
297+
// Implement safe_du on Unix (except Redox which lacks full stat support)
298+
// This is done for TOCTOU safety
299299
fn safe_du(
300300
path: &Path,
301301
options: &TraversalOptions,
@@ -439,7 +439,8 @@ fn safe_du(
439439
const S_IFMT: u32 = 0o170_000;
440440
const S_IFDIR: u32 = 0o040_000;
441441
const S_IFLNK: u32 = 0o120_000;
442-
let is_symlink = (lstat.st_mode & S_IFMT) == S_IFLNK;
442+
#[allow(clippy::unnecessary_cast)]
443+
let is_symlink = (lstat.st_mode as u32 & S_IFMT) == S_IFLNK;
443444

444445
// Handle symlinks with -L option
445446
// For safe traversal with -L, we skip symlinks to directories entirely
@@ -450,12 +451,14 @@ fn safe_du(
450451
continue;
451452
}
452453

453-
let is_dir = (lstat.st_mode & S_IFMT) == S_IFDIR;
454+
#[allow(clippy::unnecessary_cast)]
455+
let is_dir = (lstat.st_mode as u32 & S_IFMT) == S_IFDIR;
454456
let entry_stat = lstat;
455457

458+
#[allow(clippy::unnecessary_cast)]
456459
let file_info = (entry_stat.st_ino != 0).then_some(FileInfo {
457460
file_id: entry_stat.st_ino as u128,
458-
dev_id: entry_stat.st_dev,
461+
dev_id: entry_stat.st_dev as u64,
459462
});
460463

461464
// For safe traversal, we need to handle stats differently
@@ -465,6 +468,7 @@ fn safe_du(
465468
Stat {
466469
path: entry_path.clone(),
467470
size: 0,
471+
#[allow(clippy::unnecessary_cast)]
468472
blocks: entry_stat.st_blocks as u64,
469473
inodes: 1,
470474
inode: file_info,
@@ -476,7 +480,9 @@ fn safe_du(
476480
// For files
477481
Stat {
478482
path: entry_path.clone(),
483+
#[allow(clippy::unnecessary_cast)]
479484
size: entry_stat.st_size as u64,
485+
#[allow(clippy::unnecessary_cast)]
480486
blocks: entry_stat.st_blocks as u64,
481487
inodes: 1,
482488
inode: file_info,
@@ -1096,14 +1102,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
10961102
let mut seen_inodes: HashSet<FileInfo> = HashSet::new();
10971103

10981104
// Determine which traversal method to use
1099-
#[cfg(target_os = "linux")]
1105+
#[cfg(all(unix, not(target_os = "redox")))]
11001106
let use_safe_traversal = traversal_options.dereference != Deref::All;
1101-
#[cfg(not(target_os = "linux"))]
1107+
#[cfg(not(all(unix, not(target_os = "redox"))))]
11021108
let use_safe_traversal = false;
11031109

11041110
if use_safe_traversal {
1105-
// Use safe traversal (Linux only, when not using -L)
1106-
#[cfg(target_os = "linux")]
1111+
// Use safe traversal (Unix except Redox, when not using -L)
1112+
#[cfg(all(unix, not(target_os = "redox")))]
11071113
{
11081114
// Pre-populate seen_inodes with the starting directory to detect cycles
11091115
if let Ok(stat) = Stat::new(&path, None, &traversal_options) {
@@ -1158,9 +1164,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
11581164
.send(Ok(StatPrintInfo { stat, depth: 0 }))
11591165
.map_err(|e| USimpleError::new(1, e.to_string()))?;
11601166
} else {
1161-
#[cfg(target_os = "linux")]
1167+
#[cfg(unix)]
11621168
let error_msg = translate!("du-error-cannot-access", "path" => path.quote());
1163-
#[cfg(not(target_os = "linux"))]
1169+
#[cfg(not(unix))]
11641170
let error_msg =
11651171
translate!("du-error-cannot-access-no-such-file", "path" => path.quote());
11661172

src/uu/install/src/install.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ mod mode;
1010
use clap::{Arg, ArgAction, ArgMatches, Command};
1111
use file_diff::diff;
1212
use filetime::{FileTime, set_file_times};
13-
#[cfg(feature = "selinux")]
13+
#[cfg(all(feature = "selinux", target_os = "linux"))]
1414
use selinux::SecurityContext;
1515
use std::ffi::OsString;
1616
use std::fmt::Debug;
@@ -27,7 +27,7 @@ use uucore::error::{FromIo, UError, UResult, UUsageError};
2727
use uucore::fs::dir_strip_dot_for_creation;
2828
use uucore::perms::{Verbosity, VerbosityLevel, wrap_chown};
2929
use uucore::process::{getegid, geteuid};
30-
#[cfg(feature = "selinux")]
30+
#[cfg(all(feature = "selinux", target_os = "linux"))]
3131
use uucore::selinux::{
3232
SeLinuxError, contexts_differ, get_selinux_security_context, is_selinux_enabled,
3333
selinux_error_description, set_selinux_security_context,
@@ -118,7 +118,7 @@ enum InstallError {
118118
#[error("{}", translate!("install-error-extra-operand", "operand" => .0.quote(), "usage" => .1.clone()))]
119119
ExtraOperand(OsString, String),
120120

121-
#[cfg(feature = "selinux")]
121+
#[cfg(all(feature = "selinux", target_os = "linux"))]
122122
#[error("{}", .0)]
123123
SelinuxContextFailed(String),
124124
}
@@ -1030,7 +1030,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
10301030
Ok(())
10311031
}
10321032

1033-
#[cfg(feature = "selinux")]
1033+
#[cfg(all(feature = "selinux", target_os = "linux"))]
10341034
fn get_context_for_selinux(b: &Behavior) -> Option<&String> {
10351035
if b.default_context {
10361036
None
@@ -1165,7 +1165,7 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
11651165
false
11661166
}
11671167

1168-
#[cfg(feature = "selinux")]
1168+
#[cfg(all(feature = "selinux", target_os = "linux"))]
11691169
/// Sets the `SELinux` security context for install's -Z flag behavior.
11701170
///
11711171
/// This function implements the specific behavior needed for install's -Z flag,
@@ -1199,7 +1199,7 @@ pub fn set_selinux_default_context(path: &Path) -> Result<(), SeLinuxError> {
11991199
}
12001200
}
12011201

1202-
#[cfg(feature = "selinux")]
1202+
#[cfg(all(feature = "selinux", target_os = "linux"))]
12031203
/// Gets the default `SELinux` context for a path based on the system's security policy.
12041204
///
12051205
/// This function attempts to determine what the "correct" `SELinux` context should be
@@ -1255,7 +1255,7 @@ fn get_default_context_for_path(path: &Path) -> Result<Option<String>, SeLinuxEr
12551255
Ok(None)
12561256
}
12571257

1258-
#[cfg(feature = "selinux")]
1258+
#[cfg(all(feature = "selinux", target_os = "linux"))]
12591259
/// Derives an appropriate `SELinux` context based on a parent directory context.
12601260
///
12611261
/// This is a heuristic function that attempts to generate an appropriate
@@ -1293,7 +1293,7 @@ fn derive_context_from_parent(parent_context: &str) -> String {
12931293
}
12941294
}
12951295

1296-
#[cfg(feature = "selinux")]
1296+
#[cfg(all(feature = "selinux", target_os = "linux"))]
12971297
/// Helper function to collect paths that need `SELinux` context setting.
12981298
///
12991299
/// Traverses from the given starting path up to existing parent directories.
@@ -1307,7 +1307,7 @@ fn collect_paths_for_context_setting(starting_path: &Path) -> Vec<&Path> {
13071307
paths
13081308
}
13091309

1310-
#[cfg(feature = "selinux")]
1310+
#[cfg(all(feature = "selinux", target_os = "linux"))]
13111311
/// Sets the `SELinux` security context for a directory hierarchy.
13121312
///
13131313
/// This function traverses from the given starting path up to existing parent directories
@@ -1347,7 +1347,7 @@ fn set_selinux_context_for_directories(target_path: &Path, context: Option<&Stri
13471347
}
13481348
}
13491349

1350-
#[cfg(feature = "selinux")]
1350+
#[cfg(all(feature = "selinux", target_os = "linux"))]
13511351
/// Sets `SELinux` context for created directories using install's -Z default behavior.
13521352
///
13531353
/// Similar to `set_selinux_context_for_directories` but uses install's
@@ -1371,10 +1371,10 @@ pub fn set_selinux_context_for_directories_install(target_path: &Path, context:
13711371

13721372
#[cfg(test)]
13731373
mod tests {
1374-
#[cfg(feature = "selinux")]
1374+
#[cfg(all(feature = "selinux", target_os = "linux"))]
13751375
use super::derive_context_from_parent;
13761376

1377-
#[cfg(feature = "selinux")]
1377+
#[cfg(all(feature = "selinux", target_os = "linux"))]
13781378
#[test]
13791379
fn test_derive_context_from_parent() {
13801380
// Test cases: (input_context, file_type, expected_output, description)

0 commit comments

Comments
 (0)