From 6b16cc9688fe998f5dc2786f53f22ceea709e318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=E1=BA=A3=20th=E1=BA=BF=20gi=E1=BB=9Bi=20l=C3=A0=20Rust?= <90588855+naoNao89@users.noreply.github.com> Date: Sun, 26 Apr 2026 16:12:21 +0700 Subject: [PATCH] rm: fix path parsing for dot/dotdot protection (#11005) Fixes #9749: rm -rf ./ and variants silently delete current directory contents. The path_is_current_or_parent_directory() function did not match paths like "./" after clean_trailing_slashes() collapsed them. Rewrite the function to strip trailing separators, extract the last component, and check if it is "." or "..". Co-authored-by: Sylvestre Ledru Co-authored-by: naoNao89 --- src/uu/rm/src/rm.rs | 76 ++++++++++++--- tests/by-util/test_rm.rs | 206 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 268 insertions(+), 14 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 5f2b981ea..f9aba615f 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -876,21 +876,71 @@ fn prompt_file_permission_readonly(path: &Path, options: &Options, metadata: &Me } } -/// Checks if the path is referring to current or parent directory , if it is referring to current or any parent directory in the file tree e.g '/../..' , '../..' +/// Checks if the path is referring to current or parent directory. +/// +/// Strips trailing separators, then checks if the last component is "." or "..". +/// +/// Examples: +/// - ".", "./" -> true +/// - "..", "../" -> true +/// - "foo/.", "foo/.." -> true +/// - "foo/bar" -> false fn path_is_current_or_parent_directory(path: &Path) -> bool { - let path_str = os_str_as_bytes(path.as_os_str()); - let dir_separator = MAIN_SEPARATOR as u8; - if let Ok(path_bytes) = path_str { - return path_bytes == ([b'.']) - || path_bytes == ([b'.', dir_separator]) - || path_bytes == ([b'.', b'.']) - || path_bytes == ([b'.', b'.', dir_separator]) - || path_bytes.ends_with(&[dir_separator, b'.']) - || path_bytes.ends_with(&[dir_separator, b'.', b'.']) - || path_bytes.ends_with(&[dir_separator, b'.', dir_separator]) - || path_bytes.ends_with(&[dir_separator, b'.', b'.', dir_separator]); + let Ok(bytes) = os_str_as_bytes(path.as_os_str()) else { + return false; + }; + + // Strip all trailing separators + let trimmed = match bytes.iter().rposition(|&b| !is_separator(b)) { + Some(i) => &bytes[..=i], + None => return false, // all separators or empty + }; + + // Extract the last component (after the last separator) + let last = match trimmed.iter().rposition(|&b| is_separator(b)) { + Some(i) => &trimmed[i + 1..], + None => trimmed, + }; + + last == b"." || last == b".." +} + +fn is_separator(b: u8) -> bool { + b == MAIN_SEPARATOR as u8 || cfg!(windows) && b == b'/' +} + +#[cfg(test)] +mod path_tests { + use super::*; + use std::path::Path; + + #[test] + fn test_path_is_current_or_parent_directory() { + // Current directory + assert!(path_is_current_or_parent_directory(Path::new("."))); + assert!(path_is_current_or_parent_directory(Path::new("./"))); + assert!(path_is_current_or_parent_directory(Path::new(".//"))); + + // Parent directory + assert!(path_is_current_or_parent_directory(Path::new(".."))); + assert!(path_is_current_or_parent_directory(Path::new("../"))); + assert!(path_is_current_or_parent_directory(Path::new("..//"))); + + // Nested paths ending in dot/dotdot + assert!(path_is_current_or_parent_directory(Path::new("d/."))); + assert!(path_is_current_or_parent_directory(Path::new("d/./"))); + assert!(path_is_current_or_parent_directory(Path::new("d/.."))); + assert!(path_is_current_or_parent_directory(Path::new("d/../"))); + assert!(path_is_current_or_parent_directory(Path::new("a/b/."))); + assert!(path_is_current_or_parent_directory(Path::new("a/b/.."))); + + // Not current/parent directory + assert!(!path_is_current_or_parent_directory(Path::new("file"))); + assert!(!path_is_current_or_parent_directory(Path::new(".hidden"))); + assert!(!path_is_current_or_parent_directory(Path::new("..hidden"))); + assert!(!path_is_current_or_parent_directory(Path::new("dir/file"))); + assert!(!path_is_current_or_parent_directory(Path::new("..."))); } - false } // For directories finding if they are writable or not is a hassle. In Unix we can use the built-in rust crate to check mode bits. But other os don't have something similar afaik diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index c2b77ee6d..43c712e9c 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore rootlink +// spell-checker:ignore rootlink dotdot rootfile deleteme keepme topfile #![allow(clippy::stable_sort_primitive)] use std::process::Stdio; @@ -1377,3 +1377,207 @@ fn test_preserve_root_literal_root() { .stderr_contains("it is dangerous to operate recursively on '/'") .stderr_contains("use --no-preserve-root to override this failsafe"); } + +// Regression tests for Issue #9749: rm -rf ./ and variants silently delete +// current directory contents + +/// Test edge cases for dot/dotdot protection with multiple slashes +#[test] +#[cfg(not(windows))] +fn test_dot_protection_multiple_slashes() { + // Test various patterns with multiple slashes - each in a fresh scenario + let test_cases = ["./", ".//", ".///", ".//./", "../", "..//", "..//../"]; + + for case in &test_cases { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("d"); + at.touch("file1"); + at.touch("d/file2"); + + let result = ts.ucmd().arg("-rf").arg(case).fails(); + + // Verify error message - either dot protection or root protection + // depending on temp directory structure + let stderr = result.stderr_str(); + assert!( + stderr.contains("refusing to remove") || stderr.contains("dangerous to operate"), + "Expected error message for rm -rf {case}, got: {stderr}" + ); + + // CRITICAL: Verify no files were silently deleted (the #9749 bug) + assert!( + at.dir_exists("d"), + "Directory 'd' should exist after rm -rf {case}" + ); + assert!( + at.file_exists("file1"), + "file1 should exist after rm -rf {case}" + ); + assert!( + at.file_exists("d/file2"), + "d/file2 should exist after rm -rf {case}" + ); + } +} + +/// Test dot/dotdot protection in deeply nested paths +#[test] +#[cfg(not(windows))] +fn test_dot_protection_nested_paths() { + let test_cases = [ + "a/b/c/.", + "a/b/c/./", + "a/b/c/..", + "a/b/c/../", + "a/b/.", + "a/b/../", + ]; + + for case in &test_cases { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir_all("a/b/c"); + at.touch("a/b/c/file"); + at.touch("rootfile"); + + let result = ts.ucmd().arg("-rf").arg(case).fails(); + result.stderr_contains("refusing to remove"); + + // Verify structure is intact + assert!( + at.dir_exists("a/b/c"), + "a/b/c should exist after rm -rf {case}" + ); + assert!( + at.file_exists("a/b/c/file"), + "a/b/c/file should exist after rm -rf {case}" + ); + assert!( + at.file_exists("rootfile"), + "rootfile should exist after rm -rf {case}" + ); + } +} + +/// Test that normal files/directories with dots in names are NOT protected +#[test] +fn test_normal_files_with_dots_not_protected() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.touch(".hidden"); + at.touch("..double_hidden"); + at.touch("file.txt"); + at.mkdir("dir.with.dots"); + at.touch("dir.with.dots/file"); + + // These should succeed (not be blocked) + ts.ucmd().arg("-f").arg(".hidden").succeeds(); + ts.ucmd().arg("-f").arg("..double_hidden").succeeds(); + ts.ucmd().arg("-f").arg("file.txt").succeeds(); + ts.ucmd().arg("-rf").arg("dir.with.dots").succeeds(); + + assert!(!at.file_exists(".hidden")); + assert!(!at.file_exists("..double_hidden")); + assert!(!at.file_exists("file.txt")); + assert!(!at.dir_exists("dir.with.dots")); +} + +/// Test mixed valid and invalid arguments +/// +/// When rm encounters a dot/directory protection error, it should: +/// 1. Process arguments in order +/// 2. Continue with remaining arguments after reporting the error +/// 3. Report the error +#[test] +#[cfg(not(windows))] +fn test_mixed_valid_and_dot_arguments() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("deleteme"); + at.touch("deleteme/file"); + at.mkdir("d"); // Create directory for dot test + at.touch("keepme"); + + let result = ts + .ucmd() + .arg("-rf") + .arg("deleteme") + .arg("d/.") // This should be refused (in a subdir) + .arg("keepme") + .fails(); + + // Should have deleted deleteme + assert!(!at.dir_exists("deleteme")); + + // Should have refused to remove d/. + result.stderr_contains("refusing to remove"); + + // keepme should also be deleted (rm continues after errors) + assert!(!at.file_exists("keepme")); +} + +/// Test Windows-specific edge cases +#[test] +#[cfg(windows)] +fn test_dot_protection_windows_edge_cases() { + // Test Windows backslash patterns - each in a fresh scenario + let test_cases = [".\\", ".\\\\", "..\\", "..\\\\", "d\\.\\", "d\\..\\"]; + + for case in &test_cases { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("d"); + at.touch("file1"); + at.touch("d\\file2"); + + let result = ts.ucmd().arg("-rf").arg(case).fails(); + result.stderr_contains("refusing to remove"); + + // Verify no files were deleted + assert!( + at.dir_exists("d"), + "Directory 'd' should exist after rm -rf {case}", + ); + assert!( + at.file_exists("file1"), + "file1 should exist after rm -rf {case}", + ); + } +} + +/// Test symlink targets that resolve to . or .. +/// +/// Note: Symlinks to . or .. are handled differently than direct paths. +/// The symlink itself is removed, but the target (current/parent dir) is not. +/// This test verifies symlinks can be removed without affecting the actual directories. +#[cfg(unix)] +#[test] +fn test_symlink_to_dot_protection() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + at.mkdir("subdir"); + at.touch("subdir/file"); + at.touch("topfile"); + + // Create symlinks pointing to . and .. + at.symlink_dir(".", "subdir/dot_link"); + at.symlink_dir("..", "subdir/dotdot_link"); + + // Remove the symlinks themselves (without trailing slash they are just links) + ts.ucmd().arg("-f").arg("subdir/dot_link").succeeds(); + ts.ucmd().arg("-f").arg("subdir/dotdot_link").succeeds(); + + // Verify symlinks are gone but directories still exist + assert!(!at.symlink_exists("subdir/dot_link")); + assert!(!at.symlink_exists("subdir/dotdot_link")); + assert!(at.dir_exists("subdir")); + assert!(at.file_exists("subdir/file")); + assert!(at.file_exists("topfile")); +}