Skip to content

Commit 5d56638

Browse files
committed
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 rust-lang#125060
1 parent 09d9a3b commit 5d56638

File tree

2 files changed

+36
-0
lines changed

2 files changed

+36
-0
lines changed

std/src/path.rs

+13
Original file line numberDiff line numberDiff line change
@@ -1425,6 +1425,11 @@ impl PathBuf {
14251425
/// If `extension` is the empty string, [`self.extension`] will be [`None`]
14261426
/// afterwards, not `Some("")`.
14271427
///
1428+
/// # Panics
1429+
///
1430+
/// Panics if the passed extension contains a path separator (see
1431+
/// [`is_separator`]).
1432+
///
14281433
/// # Caveats
14291434
///
14301435
/// The new `extension` may contain dots and will be used in its entirety,
@@ -1470,6 +1475,14 @@ impl PathBuf {
14701475
}
14711476

14721477
fn _set_extension(&mut self, extension: &OsStr) -> bool {
1478+
for &b in extension.as_encoded_bytes() {
1479+
if b < 128 {
1480+
if is_separator(b as char) {
1481+
panic!("extension cannot contain path separators: {:?}", extension);
1482+
}
1483+
}
1484+
}
1485+
14731486
let file_stem = match self.file_stem() {
14741487
None => return false,
14751488
Some(f) => f.as_encoded_bytes(),

std/src/path/tests.rs

+23
Original file line numberDiff line numberDiff line change
@@ -1803,6 +1803,29 @@ fn test_windows_absolute() {
18031803
assert_eq!(absolute(r"COM1").unwrap().as_os_str(), Path::new(r"\\.\COM1").as_os_str());
18041804
}
18051805

1806+
#[test]
1807+
#[should_panic = "path separator"]
1808+
fn test_extension_path_sep() {
1809+
let mut path = PathBuf::from("path/to/file");
1810+
path.set_extension("d/../../../../../etc/passwd");
1811+
}
1812+
1813+
#[test]
1814+
#[should_panic = "path separator"]
1815+
#[cfg(windows)]
1816+
fn test_extension_path_sep_alternate() {
1817+
let mut path = PathBuf::from("path/to/file");
1818+
path.set_extension("d\\test");
1819+
}
1820+
1821+
#[test]
1822+
#[cfg(not(windows))]
1823+
fn test_extension_path_sep_alternate() {
1824+
let mut path = PathBuf::from("path/to/file");
1825+
path.set_extension("d\\test");
1826+
assert_eq!(path, Path::new("path/to/file.d\\test"));
1827+
}
1828+
18061829
#[bench]
18071830
#[cfg_attr(miri, ignore)] // Miri isn't fast...
18081831
fn bench_path_cmp_fast_path_buf_sort(b: &mut test::Bencher) {

0 commit comments

Comments
 (0)