From 700b3ea61ba2a37876af0a5d2d2f991aba5c7000 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 13 May 2024 14:22:45 +0200 Subject: [PATCH] Panic if `PathBuf::set_extension` would add a path separator This is likely never intended and potentially a security vulnerability if it happens. I'd guess that it's mostly literal strings that are passed to this function in practice, so I'm guessing this doesn't break anyone. CC #125060 --- library/std/src/path.rs | 13 +++++++++++++ library/std/src/path/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 79d800ff0729b..acf23d976177b 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1425,6 +1425,11 @@ impl PathBuf { /// If `extension` is the empty string, [`self.extension`] will be [`None`] /// afterwards, not `Some("")`. /// + /// # Panics + /// + /// Panics if the passed extension contains a path separator (see + /// [`is_separator`]). + /// /// # Caveats /// /// The new `extension` may contain dots and will be used in its entirety, @@ -1470,6 +1475,14 @@ impl PathBuf { } fn _set_extension(&mut self, extension: &OsStr) -> bool { + for &b in extension.as_encoded_bytes() { + if b < 128 { + if is_separator(b as char) { + panic!("extension cannot contain path separators: {:?}", extension); + } + } + } + let file_stem = match self.file_stem() { None => return false, Some(f) => f.as_encoded_bytes(), diff --git a/library/std/src/path/tests.rs b/library/std/src/path/tests.rs index fde6ed4f0c057..2d8e50d1f88e9 100644 --- a/library/std/src/path/tests.rs +++ b/library/std/src/path/tests.rs @@ -1803,6 +1803,29 @@ fn test_windows_absolute() { assert_eq!(absolute(r"COM1").unwrap().as_os_str(), Path::new(r"\\.\COM1").as_os_str()); } +#[test] +#[should_panic = "path separator"] +fn test_extension_path_sep() { + let mut path = PathBuf::from("path/to/file"); + path.set_extension("d/../../../../../etc/passwd"); +} + +#[test] +#[should_panic = "path separator"] +#[cfg(windows)] +fn test_extension_path_sep_alternate() { + let mut path = PathBuf::from("path/to/file"); + path.set_extension("d\\test"); +} + +#[test] +#[cfg(not(windows))] +fn test_extension_path_sep_alternate() { + let mut path = PathBuf::from("path/to/file"); + path.set_extension("d\\test"); + assert_eq!(path, Path::new("path/to/file.d\\test")); +} + #[bench] #[cfg_attr(miri, ignore)] // Miri isn't fast... fn bench_path_cmp_fast_path_buf_sort(b: &mut test::Bencher) {