From c0e61ed37514f739cfbf79d09a5f72b45bd48920 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 5 Nov 2024 16:43:36 -0800 Subject: [PATCH 1/7] Correctly reject device files with multi-dot filename extensions. When rejecting device names such as "CON" and "CON.txt", reject filenames with multiple-dot extensions too, such as "CON.txt.gz". --- cap-primitives/src/windows/fs/open_impl.rs | 39 +++++++++++- tests/windows-open.rs | 72 ++++++++++++++-------- 2 files changed, 86 insertions(+), 25 deletions(-) diff --git a/cap-primitives/src/windows/fs/open_impl.rs b/cap-primitives/src/windows/fs/open_impl.rs index db595385..74d67f8b 100644 --- a/cap-primitives/src/windows/fs/open_impl.rs +++ b/cap-primitives/src/windows/fs/open_impl.rs @@ -1,4 +1,5 @@ use crate::fs::{manually, OpenOptions}; +use std::ffi::OsStr; use std::path::Path; use std::{fs, io}; use windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND; @@ -11,7 +12,7 @@ pub(crate) fn open_impl( // Windows reserves several special device paths. Disallow opening any // of them. // See: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions - if let Some(stem) = path.file_stem() { + if let Some(stem) = file_prefix(path) { if let Some(stemstr) = stem.to_str() { match stemstr.to_uppercase().as_str() { "CON" | "PRN" | "AUX" | "NUL" | "COM0" | "COM1" | "COM2" | "COM3" | "COM4" @@ -27,3 +28,39 @@ pub(crate) fn open_impl( manually::open(start, path, options) } + +// TODO: Replace this with `Path::file_prefix` once that's stable. For now, +// we use a copy of the code. This code is derived from +// https://github.com/rust-lang/rust/blob/9fe9041cc8eddaed402d17aa4facb2ce8f222e95/library/std/src/path.rs#L2648 +fn file_prefix(path: &Path) -> Option<&OsStr> { + path.file_name() + .map(split_file_at_dot) + .and_then(|(before, _after)| Some(before)) +} + +// This code is derived from +// https://github.com/rust-lang/rust/blob/9fe9041cc8eddaed402d17aa4facb2ce8f222e95/library/std/src/path.rs#L340 +#[allow(unsafe_code)] +fn split_file_at_dot(file: &OsStr) -> (&OsStr, Option<&OsStr>) { + let slice = file.as_encoded_bytes(); + if slice == b".." { + return (file, None); + } + + // The unsafety here stems from converting between &OsStr and &[u8] + // and back. This is safe to do because (1) we only look at ASCII + // contents of the encoding and (2) new &OsStr values are produced + // only from ASCII-bounded slices of existing &OsStr values. + let i = match slice[1..].iter().position(|b| *b == b'.') { + Some(i) => i + 1, + None => return (file, None), + }; + let before = &slice[..i]; + let after = &slice[i + 1..]; + unsafe { + ( + OsStr::from_encoded_bytes_unchecked(before), + Some(OsStr::from_encoded_bytes_unchecked(after)), + ) + } +} diff --git a/tests/windows-open.rs b/tests/windows-open.rs index 0cb43d8c..6a6d7220 100644 --- a/tests/windows-open.rs +++ b/tests/windows-open.rs @@ -134,30 +134,54 @@ fn windows_open_special() { // Opening any of these should fail. for device in &[ "CON", "PRN", "AUX", "NUL", "COM0", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", - "COM8", "COM9", "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", - "LPT9", + "COM8", "COM9", "COM¹", "COM²", "COM³", "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", + "LPT6", "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³", ] { - tmpdir.open(device).unwrap_err(); - tmpdir.open(&format!(".\\{}", device)).unwrap_err(); - tmpdir.open(&format!("{}.ext", device)).unwrap_err(); - tmpdir.open(&format!(".\\{}.ext", device)).unwrap_err(); - - let mut options = cap_std::fs::OpenOptions::new(); - options.write(true); - tmpdir.open_with(device, &options).unwrap_err(); - tmpdir - .open_with(&format!(".\\{}", device), &options) - .unwrap_err(); - tmpdir - .open_with(&format!("{}.ext", device), &options) - .unwrap_err(); - tmpdir - .open_with(&format!(".\\{}.ext", device), &options) - .unwrap_err(); - - tmpdir.create(device).unwrap_err(); - tmpdir.create(&format!(".\\{}", device)).unwrap_err(); - tmpdir.create(&format!("{}.ext", device)).unwrap_err(); - tmpdir.create(&format!(".\\{}.ext", device)).unwrap_err(); + for prefix in &["", " ", ".", " .", ". ", ".\\", ".\\.", ". \\.", ".\\ ."] { + for suffix in &[ + "", + " ", + ".", + ". ", + ".ext", + ".ext.", + ".ext. ", + ".ext ", + ".ext.more", + ".ext.more.", + ".ext.more ", + ".ext.more. ", + ".ext.more .", + ] { + match tmpdir + .open(&format!("{}{}{}", prefix, device, suffix)) + .unwrap_err() + .kind() + { + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} + kind => panic!("unexpected error: {:?}", kind), + } + + let mut options = cap_std::fs::OpenOptions::new(); + options.write(true); + match tmpdir + .open_with(&format!("{}{}{}", prefix, device, suffix), &options) + .unwrap_err() + .kind() + { + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} + kind => panic!("unexpected error: {:?}", kind), + } + + match tmpdir + .create(&format!("{}{}{}", prefix, device, suffix)) + .unwrap_err() + .kind() + { + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} + kind => panic!("unexpected error: {:?}", kind), + } + } + } } } From cb47341e9b9583b0324bc2b9d2839cb1ed3b273b Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 2 Dec 2024 16:03:11 -0800 Subject: [PATCH 2/7] Debugging for a test. --- tests/windows-open.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/windows-open.rs b/tests/windows-open.rs index 6a6d7220..fd9aba76 100644 --- a/tests/windows-open.rs +++ b/tests/windows-open.rs @@ -153,8 +153,11 @@ fn windows_open_special() { ".ext.more. ", ".ext.more .", ] { + let name = format!("{}{}{}", prefix, device, suffix); + eprintln!("testing '{}'", name); + match tmpdir - .open(&format!("{}{}{}", prefix, device, suffix)) + .open(&name) .unwrap_err() .kind() { @@ -165,7 +168,7 @@ fn windows_open_special() { let mut options = cap_std::fs::OpenOptions::new(); options.write(true); match tmpdir - .open_with(&format!("{}{}{}", prefix, device, suffix), &options) + .open_with(&name, &options) .unwrap_err() .kind() { @@ -174,7 +177,7 @@ fn windows_open_special() { } match tmpdir - .create(&format!("{}{}{}", prefix, device, suffix)) + .create(&name) .unwrap_err() .kind() { From 087035f4a9dd43e70600bb780bbeb4187cc48ef1 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 2 Dec 2024 16:15:09 -0800 Subject: [PATCH 3/7] Strip trailing whitespace too. --- cap-primitives/src/windows/fs/open_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cap-primitives/src/windows/fs/open_impl.rs b/cap-primitives/src/windows/fs/open_impl.rs index 74d67f8b..a2695e8a 100644 --- a/cap-primitives/src/windows/fs/open_impl.rs +++ b/cap-primitives/src/windows/fs/open_impl.rs @@ -14,7 +14,7 @@ pub(crate) fn open_impl( // See: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions if let Some(stem) = file_prefix(path) { if let Some(stemstr) = stem.to_str() { - match stemstr.to_uppercase().as_str() { + match stemstr.trim_end().to_uppercase().as_str() { "CON" | "PRN" | "AUX" | "NUL" | "COM0" | "COM1" | "COM2" | "COM3" | "COM4" | "COM5" | "COM6" | "COM7" | "COM8" | "COM9" | "COM¹" | "COM²" | "COM³" | "LPT0" | "LPT1" | "LPT2" | "LPT3" | "LPT4" | "LPT5" | "LPT6" | "LPT7" From 0ab283f6c23a6dbee78dc777570237c194003009 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 2 Dec 2024 16:15:22 -0800 Subject: [PATCH 4/7] rustfmt --- tests/windows-open.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/tests/windows-open.rs b/tests/windows-open.rs index fd9aba76..9b51210a 100644 --- a/tests/windows-open.rs +++ b/tests/windows-open.rs @@ -156,31 +156,19 @@ fn windows_open_special() { let name = format!("{}{}{}", prefix, device, suffix); eprintln!("testing '{}'", name); - match tmpdir - .open(&name) - .unwrap_err() - .kind() - { + match tmpdir.open(&name).unwrap_err().kind() { std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} kind => panic!("unexpected error: {:?}", kind), } let mut options = cap_std::fs::OpenOptions::new(); options.write(true); - match tmpdir - .open_with(&name, &options) - .unwrap_err() - .kind() - { + match tmpdir.open_with(&name, &options).unwrap_err().kind() { std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} kind => panic!("unexpected error: {:?}", kind), } - match tmpdir - .create(&name) - .unwrap_err() - .kind() - { + match tmpdir.create(&name).unwrap_err().kind() { std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} kind => panic!("unexpected error: {:?}", kind), } From 51591178884b3bc2f9878b8d3fc706d8f9ead079 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 2 Dec 2024 16:20:04 -0800 Subject: [PATCH 5/7] Remove tests with whitespace before the device name. --- tests/windows-open.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/windows-open.rs b/tests/windows-open.rs index 9b51210a..59d3e010 100644 --- a/tests/windows-open.rs +++ b/tests/windows-open.rs @@ -137,7 +137,7 @@ fn windows_open_special() { "COM8", "COM9", "COM¹", "COM²", "COM³", "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³", ] { - for prefix in &["", " ", ".", " .", ". ", ".\\", ".\\.", ". \\.", ".\\ ."] { + for prefix in &["", ".", " .", ".\\", ".\\.", ". \\.", ".\\ ."] { for suffix in &[ "", " ", From feaeb8135239419e4c7e1642d86bc3c6b01450b0 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 2 Dec 2024 16:30:01 -0800 Subject: [PATCH 6/7] Remove prefixes entirely. --- tests/windows-open.rs | 68 +++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/tests/windows-open.rs b/tests/windows-open.rs index 59d3e010..7bc52368 100644 --- a/tests/windows-open.rs +++ b/tests/windows-open.rs @@ -137,41 +137,39 @@ fn windows_open_special() { "COM8", "COM9", "COM¹", "COM²", "COM³", "LPT0", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³", ] { - for prefix in &["", ".", " .", ".\\", ".\\.", ". \\.", ".\\ ."] { - for suffix in &[ - "", - " ", - ".", - ". ", - ".ext", - ".ext.", - ".ext. ", - ".ext ", - ".ext.more", - ".ext.more.", - ".ext.more ", - ".ext.more. ", - ".ext.more .", - ] { - let name = format!("{}{}{}", prefix, device, suffix); - eprintln!("testing '{}'", name); - - match tmpdir.open(&name).unwrap_err().kind() { - std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} - kind => panic!("unexpected error: {:?}", kind), - } - - let mut options = cap_std::fs::OpenOptions::new(); - options.write(true); - match tmpdir.open_with(&name, &options).unwrap_err().kind() { - std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} - kind => panic!("unexpected error: {:?}", kind), - } - - match tmpdir.create(&name).unwrap_err().kind() { - std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} - kind => panic!("unexpected error: {:?}", kind), - } + for suffix in &[ + "", + " ", + ".", + ". ", + ".ext", + ".ext.", + ".ext. ", + ".ext ", + ".ext.more", + ".ext.more.", + ".ext.more ", + ".ext.more. ", + ".ext.more .", + ] { + let name = format!("{}{}{}", device, suffix); + eprintln!("testing '{}'", name); + + match tmpdir.open(&name).unwrap_err().kind() { + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} + kind => panic!("unexpected error: {:?}", kind), + } + + let mut options = cap_std::fs::OpenOptions::new(); + options.write(true); + match tmpdir.open_with(&name, &options).unwrap_err().kind() { + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} + kind => panic!("unexpected error: {:?}", kind), + } + + match tmpdir.create(&name).unwrap_err().kind() { + std::io::ErrorKind::NotFound | std::io::ErrorKind::PermissionDenied => {} + kind => panic!("unexpected error: {:?}", kind), } } } From d692643958a2543e9f70c89afcab5487fe737c0b Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 2 Dec 2024 16:44:27 -0800 Subject: [PATCH 7/7] Fix. --- tests/windows-open.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/windows-open.rs b/tests/windows-open.rs index 7bc52368..a6c8b2bb 100644 --- a/tests/windows-open.rs +++ b/tests/windows-open.rs @@ -152,7 +152,7 @@ fn windows_open_special() { ".ext.more. ", ".ext.more .", ] { - let name = format!("{}{}{}", device, suffix); + let name = format!("{}{}", device, suffix); eprintln!("testing '{}'", name); match tmpdir.open(&name).unwrap_err().kind() {