From f72504301ef1278831a4afcb84c32cb24bde7289 Mon Sep 17 00:00:00 2001 From: just-an-engineer Date: Wed, 7 Aug 2024 11:49:20 -0400 Subject: [PATCH 1/3] refuse to remove '.' and '..' Co-authored-by: Anirban Halder --- src/uu/rm/src/rm.rs | 18 ++++++++++++++++++ tests/by-util/test_rm.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index a89ba6db67f..3b503b5519f 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -326,6 +326,24 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool { fn handle_dir(path: &Path, options: &Options) -> bool { let mut had_err = false; + let path_as_str = path.as_os_str().to_str().unwrap(); + #[cfg(unix)] + if path_as_str.ends_with("/.") || path_as_str.ends_with("/..") { + show_error!( + "refusing to remove '.' or '..' directory: skipping {}", + path.quote() + ); + return true; + } + #[cfg(windows)] + if path_as_str.ends_with("\\.") || path_as_str.ends_with("\\..") { + show_error!( + "refusing to remove '.' or '..' directory: skipping {}", + path.quote() + ); + return true; + } + let is_root = path.has_root() && path.parent().is_none(); if options.recursive && (!is_root || !options.preserve_root) { if options.interactive != InteractiveMode::Always && !options.verbose { diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index f997688c818..75fb84bfd71 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -713,3 +713,37 @@ fn test_non_utf8() { ucmd.arg(file).succeeds(); assert!(!at.file_exists(file)); } + +#[test] +fn test_rm_no_del_parent() { + let (at, mut ucmd) = at_and_ucmd!(); + + #[cfg(not(windows))] + let del = "/"; + #[cfg(windows)] + let del = "\\"; + + at.mkdir("test"); + at.mkdir(&format!("test{del}dir")); + at.touch(&format!("test{del}dir{del}file")); + ucmd.arg("-rf").arg(format!("test{del}dir{del}..")).fails(); + assert!(at.dir_exists("test")); + assert!(at.dir_exists(&format!("test{del}dir"))); + assert!(at.file_exists(&format!("test{del}dir{del}file"))); +} + +#[test] +fn test_rm_no_del_cur() { + let (at, mut ucmd) = at_and_ucmd!(); + + #[cfg(not(windows))] + let del = "/"; + #[cfg(windows)] + let del = "\\"; + + at.mkdir("test"); + at.touch(&format!("test{del}file")); + ucmd.arg("-rf").arg(format!("test{del}.")).fails(); + assert!(at.dir_exists("test")); + assert!(at.file_exists(&format!("test{del}file"))); +} From 6f112aade3a27f4c950df9e59d2c17a6ed34ea7b Mon Sep 17 00:00:00 2001 From: just-an-engineer Date: Thu, 8 Aug 2024 11:16:51 -0400 Subject: [PATCH 2/3] Simplified parent/cur dir deletion avoidance in `rm` Co-authored-by: Anirban Halder --- src/uu/rm/src/rm.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 3b503b5519f..44e8c247edc 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -326,17 +326,20 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool { fn handle_dir(path: &Path, options: &Options) -> bool { let mut had_err = false; - let path_as_str = path.as_os_str().to_str().unwrap(); #[cfg(unix)] - if path_as_str.ends_with("/.") || path_as_str.ends_with("/..") { - show_error!( - "refusing to remove '.' or '..' directory: skipping {}", - path.quote() - ); - return true; - } + let del = "/"; #[cfg(windows)] - if path_as_str.ends_with("\\.") || path_as_str.ends_with("\\..") { + let del = "\\"; + + if path + .to_str() + .unwrap() + .ends_with(format!("{}.", del).as_str()) + || path + .to_str() + .unwrap() + .ends_with(format!("{}..", del).as_str()) + { show_error!( "refusing to remove '.' or '..' directory: skipping {}", path.quote() From d894f910cba37d9d99ba68792747f709fd8a8347 Mon Sep 17 00:00:00 2001 From: just-an-engineer Date: Sat, 7 Sep 2024 15:31:56 -0400 Subject: [PATCH 3/3] Use MAIN_SERPARATOR over manual conditional variable assignment --- src/uu/rm/src/rm.rs | 5 +---- tests/by-util/test_rm.rs | 10 ++-------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 44e8c247edc..a195ea7bb97 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -326,10 +326,7 @@ pub fn remove(files: &[&OsStr], options: &Options) -> bool { fn handle_dir(path: &Path, options: &Options) -> bool { let mut had_err = false; - #[cfg(unix)] - let del = "/"; - #[cfg(windows)] - let del = "\\"; + let del = std::path::MAIN_SEPARATOR; if path .to_str() diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index 75fb84bfd71..3ddb99015b9 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -718,10 +718,7 @@ fn test_non_utf8() { fn test_rm_no_del_parent() { let (at, mut ucmd) = at_and_ucmd!(); - #[cfg(not(windows))] - let del = "/"; - #[cfg(windows)] - let del = "\\"; + let del = std::path::MAIN_SEPARATOR; at.mkdir("test"); at.mkdir(&format!("test{del}dir")); @@ -736,10 +733,7 @@ fn test_rm_no_del_parent() { fn test_rm_no_del_cur() { let (at, mut ucmd) = at_and_ucmd!(); - #[cfg(not(windows))] - let del = "/"; - #[cfg(windows)] - let del = "\\"; + let del = std::path::MAIN_SEPARATOR; at.mkdir("test"); at.touch(&format!("test{del}file"));