Skip to content

Commit 13ea1fc

Browse files
authored
chmod:fix safe traversal/access (#9554)
* feat(chmod): use dirfd for recursive subdirectory traversal - Update chmod recursive logic to use directory file descriptors instead of full paths for subdirectories - Improves performance, avoids path length issues, and ensures dirfd-relative openat calls - Add test to verify strace output shows no AT_FDCWD with multi-component paths * test(chmod): add spell-check ignore for dirfd, subdirs, openat, FDCWD Added a spell-checker ignore directive in the chmod test file to suppress false positives for legitimate technical terms used in Unix API calls. * test(chmod): enforce strace requirement in recursive test, fail fast instead of skip Previously, the test_chmod_recursive_uses_dirfd_for_subdirs test skipped gracefully if strace was unavailable, without failing. This change enforces the strace dependency by failing the test immediately if strace is not installed or runnable, ensuring the test runs reliably in environments where it is expected to pass, and preventing silent skips. * ci: install strace in Ubuntu CI jobs for debugging system calls Add installation of strace tool on Ubuntu runners in both individual build/test and feature build/test jobs. This enables tracing system calls during execution, aiding in debugging and performance analysis within the CI/CD pipeline. Updated existing apt-get commands and added conditional steps for Linux-only installations. * ci: Add strace installation to Ubuntu-based CI workflows Install strace on ubuntu-latest runners across multiple jobs to enable system call tracing for testing purposes, ensuring compatibility with tests that require this debugging tool. This includes updating package lists in existing installation steps. * chore(build): install strace and prevent apt prompts in Cross.toml pre-build Modified the pre-build command to install strace utility for debugging and added -y flag to apt-get install to skip prompts, ensuring non-interactive builds. * feat(build): support Alpine-based cross images in pre-build Detect package manager (apt vs apk) to install tzdata and strace in both Debian/Ubuntu and Alpine *-musl targets. Added fallback warning for unsupported managers. This ensures strace is available for targets using Alpine, which doesn't have apt-get. * refactor(build): improve pre-build script readability by using multi-line strings Replace escaped multi-line string with triple-quoted string for better readability in Cross.toml. * feat(ci): install strace in WSL2 GitHub Actions workflow Install strace utility in the WSL2 environment to support tracing system calls during testing. Minor update to Cross.toml spell-checker ignore list for consistency with change. * ci(wsl2): install strace as root with non-interactive apt-get Updated the WSL2 workflow step to use root shell (wsl-bash-root) for installing strace, removing sudo calls and adding DEBIAN_FRONTEND=noninteractive to prevent prompts. This improves CI reliability by ensuring direct root access and automated, interrupt-free package installation. * ci: Move strace installation to user shell and update spell ignore Fix WSL2 GitHub Actions workflow by installing strace as the user instead of root for better permission handling, and add "noninteractive" to the spell-checker ignore comment for consistency with the new apt-get command. This ensures the tool is available in the testing environment without unnecessary privilege escalation. * chore: ci: remove unused strace installation from CI workflows Remove strace package installation from multiple GitHub Actions workflow files (CICD.yml, l10n.yml, wsl2.yml). Strace was historically installed in Ubuntu jobs for debugging system calls, but it's no longer required for the tests and builds, reducing CI setup time and dependencies. * ci: add strace installation and fix spell-checker comments in CI files - Install strace package in CICD workflow to support safe traversal verification for utilities like rm, chmod, chown, chgrp, mv, and du, enabling syscall tracing for testing. - Clean up spell-checker ignore comments in wsl2.yml and Cross.toml by removing misplaced flags.第二个测试产品**ci: add strace installation and fix spell-checker comments in CI files** - Install strace package in CICD workflow to support safe traversal verification for utilities like rm, chmod, chown, chgrp, mv, and du, enabling syscall tracing for testing. - Clean up spell-checker ignore comments in wsl2.yml and Cross.toml by removing misplaced flags. * test: add regression guard for recursive chmod dirfd-relative traversal Add a check in check-safe-traversal.sh to ensure recursive chmod operations use dirfd-relative openat calls instead of AT_FDCWD with multi-component paths, preventing potential race conditions. Ignore the corresponding Rust test as it is now covered by this shell script guard.
1 parent 796478b commit 13ea1fc

File tree

3 files changed

+68
-2
lines changed

3 files changed

+68
-2
lines changed

src/uu/chmod/src/chmod.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,9 +522,24 @@ impl Chmoder {
522522
.safe_chmod_file(&entry_path, dir_fd, &entry_name, meta.mode() & 0o7777)
523523
.and(r);
524524

525-
// Recurse into subdirectories
525+
// Recurse into subdirectories using the existing directory fd
526526
if meta.is_dir() {
527-
r = self.walk_dir_with_context(&entry_path, false).and(r);
527+
match dir_fd.open_subdir(&entry_name) {
528+
Ok(child_dir_fd) => {
529+
r = self.safe_traverse_dir(&child_dir_fd, &entry_path).and(r);
530+
}
531+
Err(err) => {
532+
let error = if err.kind() == std::io::ErrorKind::PermissionDenied {
533+
ChmodError::PermissionDenied(
534+
entry_path.to_string_lossy().to_string(),
535+
)
536+
.into()
537+
} else {
538+
err.into()
539+
};
540+
r = r.and(Err(error));
541+
}
542+
}
528543
}
529544
}
530545
}

tests/by-util/test_chmod.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
5+
// spell-checker:ignore (words) dirfd subdirs openat FDCWD
56

67
use std::fs::{OpenOptions, Permissions, metadata, set_permissions};
78
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
@@ -1280,6 +1281,51 @@ fn test_chmod_non_utf8_paths() {
12801281
);
12811282
}
12821283

1284+
#[cfg(all(target_os = "linux", feature = "chmod"))]
1285+
#[test]
1286+
#[ignore = "covered by util/check-safe-traversal.sh"]
1287+
fn test_chmod_recursive_uses_dirfd_for_subdirs() {
1288+
use std::process::Command;
1289+
use uutests::get_tests_binary;
1290+
1291+
// strace is required; fail fast if it is missing or not runnable
1292+
let output = Command::new("strace")
1293+
.arg("-V")
1294+
.output()
1295+
.expect("strace not found; install strace to run this test");
1296+
assert!(
1297+
output.status.success(),
1298+
"strace -V failed; ensure strace is installed and usable"
1299+
);
1300+
1301+
let (at, _ucmd) = at_and_ucmd!();
1302+
at.mkdir("x");
1303+
at.mkdir("x/y");
1304+
at.mkdir("x/y/z");
1305+
1306+
let log_path = at.plus_as_string("strace.log");
1307+
1308+
let status = Command::new("strace")
1309+
.arg("-e")
1310+
.arg("openat")
1311+
.arg("-o")
1312+
.arg(&log_path)
1313+
.arg(get_tests_binary!())
1314+
.args(["chmod", "-R", "+x", "x"])
1315+
.current_dir(&at.subdir)
1316+
.status()
1317+
.expect("failed to run strace");
1318+
assert!(status.success(), "strace run failed");
1319+
1320+
let log = at.read("strace.log");
1321+
1322+
// Regression guard: ensure recursion uses dirfd-relative openat instead of AT_FDCWD with a multi-component path
1323+
assert!(
1324+
!log.contains("openat(AT_FDCWD, \"x/y"),
1325+
"chmod recursed using AT_FDCWD with a multi-component path; expected dirfd-relative openat"
1326+
);
1327+
}
1328+
12831329
#[test]
12841330
fn test_chmod_colored_output() {
12851331
// Test colored help message

util/check-safe-traversal.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,11 @@ fi
173173
if echo "$AVAILABLE_UTILS" | grep -q "chmod"; then
174174
cp -r test_dir test_chmod
175175
check_utility "chmod" "openat,fchmodat,newfstatat,chmod" "openat fchmodat" "-R 755 test_chmod" "recursive_chmod"
176+
177+
# Additional regression guard: ensure recursion uses dirfd-relative openat, not AT_FDCWD with a multi-component path
178+
if grep -q 'openat(AT_FDCWD, "test_chmod/' strace_chmod_recursive_chmod.log; then
179+
fail_immediately "chmod recursed using AT_FDCWD with a multi-component path; expected dirfd-relative openat"
180+
fi
176181
fi
177182

178183
# Test chown - should use openat, fchownat, newfstatat

0 commit comments

Comments
 (0)