From 700b3ea61ba2a37876af0a5d2d2f991aba5c7000 Mon Sep 17 00:00:00 2001
From: Tobias Bucher <tobiasbucher5991@gmail.com>
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) {