Skip to content

Commit a203c65

Browse files
committed
chmod: re-check --preserve-root during the recursive descent
--preserve-root was only enforced for the paths named on the command line (the operand loop in Chmoder::chmod). With -R -L, a symlink encountered *inside* the tree that resolves to / was followed, and the recursion walked into the real root, defeating the failsafe. Reproduced in a sandboxed root: a tree containing `link -> /` with `chmod -R -L --preserve-root 777 tree` took an unrelated 0700 directory to 0777. GNU coreutils 9.10 refuses the same case. Re-check the guard at every descent, in both walk_dir_with_context variants so unix, non-unix and redox are all covered. Only symlinks are canonicalized, so ordinary recursive trees are unaffected. Behaviour now matches GNU: diagnose with the "(same as '/')" wording, skip the subtree, continue, and exit 1 -- rather than aborting the whole run. chown/chgrp already did this in 5c2c38c; chmod was the outlier. PR #10033 fixed only the operand-level "resolves to /" case.
1 parent 0e791b1 commit a203c65

4 files changed

Lines changed: 49 additions & 0 deletions

File tree

src/uu/chmod/locales/en-US.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ chmod-error-dangling-symlink = cannot operate on dangling symlink {$file}
99
chmod-error-no-such-file = cannot access {$file}: No such file or directory
1010
chmod-error-preserve-root = it is dangerous to operate recursively on {$file}
1111
chmod: use --no-preserve-root to override this failsafe
12+
chmod-error-preserve-root-same-as = it is dangerous to operate recursively on {$file} (same as '/')
13+
chmod: use --no-preserve-root to override this failsafe
1214
chmod-error-permission-denied = cannot access {$file}: Permission denied
1315
chmod-error-new-permissions = {$file}: new permissions are {$actual}, not {$expected}
1416
chmod-error-changing-permissions = changing permissions of {$file}: {$err}

src/uu/chmod/locales/fr-FR.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ chmod-error-dangling-symlink = impossible d'opérer sur le lien symbolique pendo
2121
chmod-error-no-such-file = impossible d'accéder à {$file} : Aucun fichier ou répertoire de ce type
2222
chmod-error-preserve-root = il est dangereux d'opérer récursivement sur {$file}
2323
chmod: utiliser --no-preserve-root pour outrepasser cette protection
24+
chmod-error-preserve-root-same-as = il est dangereux d'opérer récursivement sur {$file} (identique à '/')
25+
chmod: utiliser --no-preserve-root pour outrepasser cette protection
2426
chmod-error-permission-denied = impossible d'accéder à {$file} : Permission refusée
2527
chmod-error-new-permissions = {$file} : les nouvelles permissions sont {$actual}, pas {$expected}
2628
chmod-error-changing-permissions = changement des permissions de {$file} : {$err}

src/uu/chmod/src/chmod.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ enum ChmodError {
3636
NoSuchFile(PathBuf),
3737
#[error("{}", translate!("chmod-error-preserve-root", "file" => _0.quote()))]
3838
PreserveRoot(PathBuf),
39+
#[error("{}", translate!("chmod-error-preserve-root-same-as", "file" => _0.quote()))]
40+
PreserveRootSameAs(PathBuf),
3941
#[error("{}", translate!("chmod-error-permission-denied", "file" => _0.quote()))]
4042
PermissionDenied(PathBuf),
4143
#[error("{}", translate!("chmod-error-new-permissions", "file" => _0.maybe_quote(), "actual" => _1.clone(), "expected" => _2.clone()))]
@@ -424,6 +426,17 @@ impl Chmoder {
424426
matches!(fs::canonicalize(&file), Ok(p) if p == Path::new("/"))
425427
}
426428

429+
/// `--preserve-root` guard for the recursive descent.
430+
///
431+
/// The operand loop in [`Self::chmod`] only checks the paths named on the
432+
/// command line. With `-L`, a symlink met *inside* the tree can resolve to
433+
/// `/`, so the failsafe has to be re-checked at every descent or the
434+
/// recursion walks straight into the real root. Only symlinks are
435+
/// canonicalized, so ordinary trees pay nothing for this.
436+
fn descends_into_root(&self, path: &Path) -> bool {
437+
self.preserve_root && path.is_symlink() && Self::is_root(path)
438+
}
439+
427440
// Non-safe traversal implementation for platforms without safe_traversal support
428441
#[cfg(any(not(unix), target_os = "redox"))]
429442
fn walk_dir_with_context(
@@ -432,6 +445,12 @@ impl Chmoder {
432445
is_command_line_arg: bool,
433446
ancestors: &mut HashSet<FileInformation>,
434447
) -> UResult<()> {
448+
// Skip (and diagnose) a symlink that resolves to '/' before touching it.
449+
if self.descends_into_root(file_path) {
450+
show!(ChmodError::PreserveRootSameAs(file_path.into()));
451+
return Ok(());
452+
}
453+
435454
let mut r = self.chmod_file(file_path);
436455

437456
// Determine whether to traverse symlinks based on context and traversal mode
@@ -503,6 +522,12 @@ impl Chmoder {
503522
is_command_line_arg: bool,
504523
ancestors: &mut HashSet<FileInformation>,
505524
) -> UResult<()> {
525+
// Skip (and diagnose) a symlink that resolves to '/' before touching it.
526+
if self.descends_into_root(file_path) {
527+
show!(ChmodError::PreserveRootSameAs(file_path.into()));
528+
return Ok(());
529+
}
530+
506531
let mut r = self.chmod_file(file_path);
507532

508533
// Determine whether to traverse symlinks based on context and traversal mode

tests/by-util/test_chmod.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,26 @@ fn test_chmod_preserve_root_with_paths_that_resolve_to_root() {
550550
.stderr_contains("chmod: it is dangerous to operate recursively on '/'");
551551
}
552552

553+
#[test]
554+
fn test_chmod_preserve_root_symlink_during_recursion() {
555+
// The failsafe must be re-checked during the descent, not only for the
556+
// operands: with -L, a symlink met inside the tree that resolves to '/'
557+
// would otherwise be recursed into.
558+
let (at, mut ucmd) = at_and_ucmd!();
559+
at.mkdir("tree");
560+
at.symlink_dir("/", "tree/link");
561+
562+
ucmd.arg("-R")
563+
.arg("-L")
564+
.arg("--preserve-root")
565+
.arg("755")
566+
.arg("tree")
567+
.fails_with_code(1)
568+
.stderr_contains(
569+
"chmod: it is dangerous to operate recursively on 'tree/link' (same as '/')",
570+
);
571+
}
572+
553573
#[test]
554574
fn test_chmod_symlink_non_existing_file() {
555575
let scene = TestScenario::new(util_name!());

0 commit comments

Comments
 (0)