mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Fix Windows 10 drive-letter opens for no-follow filesystem operations (#51511)
## Why On Windows 10, strict native opens can reject the DOS drive alias itself as a reparse point, preventing no-follow filesystem operations on ordinary drive-letter paths. ## What changed Retry rejected drive-letter opens relative to a verified local volume root. Require the root to resolve to a physical disk volume directory without reparse attributes, and retain `OBJ_DONT_REPARSE` for paths beneath it so junctions remain blocked. ## Testing Add Windows tests for file creation and reading through the volume fallback, opening the drive root, and rejecting junction traversal and creation. Validate accepted volume-root names and verify that named pipes are rejected without establishing a client connection. GitOrigin-RevId: 51ffde4f77d2af45d18dfc4cb026d73a0e69b061
This commit is contained in:
@@ -4,6 +4,7 @@ use std::ffi::c_void;
|
||||
use std::io;
|
||||
use std::io::Write;
|
||||
use std::mem::size_of;
|
||||
use std::mem::size_of_val;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use std::os::windows::io::FromRawHandle;
|
||||
@@ -36,6 +37,9 @@ use windows_sys::Win32::Storage::FileSystem::SetFileInformationByHandle;
|
||||
use windows_sys::Win32::System::IO::IO_STATUS_BLOCK;
|
||||
use windows_sys::Win32::System::IO::IO_STATUS_BLOCK_0;
|
||||
|
||||
#[path = "windows_volume_fallback.rs"]
|
||||
mod volume_fallback;
|
||||
|
||||
const FILE_DIRECTORY_FILE: u32 = 0x0000_0001;
|
||||
const FILE_SYNCHRONOUS_IO_NONALERT: u32 = 0x0000_0020;
|
||||
const FILE_NON_DIRECTORY_FILE: u32 = 0x0000_0040;
|
||||
@@ -125,13 +129,50 @@ fn open_handle(
|
||||
create_disposition: u32,
|
||||
create_options: u32,
|
||||
) -> io::Result<OwnedHandle> {
|
||||
let mut path = nt_path(path)?;
|
||||
let name_length = u16::try_from((path.len() - 1) * size_of::<u16>())
|
||||
.map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "filesystem path is too long"))?;
|
||||
let maximum_length = u16::try_from(path.len() * size_of::<u16>())
|
||||
let mut name = nt_path(path)?;
|
||||
if let Some(handle) = open_nt_handle(
|
||||
&mut name,
|
||||
/*root_directory*/ None,
|
||||
desired_access,
|
||||
create_disposition,
|
||||
create_options,
|
||||
OBJ_DONT_REPARSE,
|
||||
)? {
|
||||
return Ok(handle);
|
||||
}
|
||||
|
||||
// Windows 10 also rejects the DOS drive alias itself. Only for drive-letter
|
||||
// paths, open its verified volume root and keep strict checks below it.
|
||||
if matches!(path.components().next(), Some(Component::Prefix(prefix))
|
||||
if matches!(prefix.kind(), Prefix::Disk(_) | Prefix::VerbatimDisk(_)))
|
||||
{
|
||||
return volume_fallback::open_relative_to_volume(
|
||||
&mut name,
|
||||
desired_access,
|
||||
create_disposition,
|
||||
create_options,
|
||||
);
|
||||
}
|
||||
Err(reparse_error())
|
||||
}
|
||||
|
||||
fn reparse_error() -> io::Error {
|
||||
io::Error::new(io::ErrorKind::InvalidInput, "path contains a reparse point")
|
||||
}
|
||||
|
||||
/// Returns `None` only for the native reparse status; other errors propagate.
|
||||
fn open_nt_handle(
|
||||
path: &mut [u16],
|
||||
root_directory: Option<&OwnedHandle>,
|
||||
desired_access: u32,
|
||||
create_disposition: u32,
|
||||
create_options: u32,
|
||||
attributes: u32,
|
||||
) -> io::Result<Option<OwnedHandle>> {
|
||||
let maximum_length = u16::try_from(size_of_val(path))
|
||||
.map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "filesystem path is too long"))?;
|
||||
let object_name = UNICODE_STRING {
|
||||
Length: name_length,
|
||||
Length: maximum_length - size_of::<u16>() as u16,
|
||||
MaximumLength: maximum_length,
|
||||
Buffer: path.as_mut_ptr(),
|
||||
};
|
||||
@@ -143,9 +184,9 @@ fn open_handle(
|
||||
};
|
||||
let object_attributes = ObjectAttributes {
|
||||
length: size_of::<ObjectAttributes>() as u32,
|
||||
root_directory: ptr::null_mut(),
|
||||
root_directory: root_directory.map_or(ptr::null_mut(), AsRawHandle::as_raw_handle),
|
||||
object_name: &object_name,
|
||||
attributes: OBJ_CASE_INSENSITIVE | OBJ_DONT_REPARSE,
|
||||
attributes: OBJ_CASE_INSENSITIVE | attributes,
|
||||
security_descriptor: ptr::null(),
|
||||
security_quality_of_service: (&raw const security_quality_of_service).cast(),
|
||||
};
|
||||
@@ -169,13 +210,10 @@ fn open_handle(
|
||||
/*ea_length*/ 0,
|
||||
)
|
||||
};
|
||||
if status == STATUS_REPARSE_POINT_ENCOUNTERED {
|
||||
return Ok(None);
|
||||
}
|
||||
if status < 0 {
|
||||
if status == STATUS_REPARSE_POINT_ENCOUNTERED {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"path contains a reparse point",
|
||||
));
|
||||
}
|
||||
let code = unsafe { RtlNtStatusToDosError(status) };
|
||||
return Err(io::Error::from_raw_os_error(code as i32));
|
||||
}
|
||||
@@ -185,7 +223,7 @@ fn open_handle(
|
||||
));
|
||||
}
|
||||
|
||||
Ok(unsafe { OwnedHandle::from_raw_handle(handle) })
|
||||
Ok(Some(unsafe { OwnedHandle::from_raw_handle(handle) }))
|
||||
}
|
||||
|
||||
fn open_entry(path: &Path) -> io::Result<std::fs::File> {
|
||||
|
||||
@@ -1,10 +1,97 @@
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use windows_sys::Win32::Foundation::ERROR_PIPE_LISTENING;
|
||||
use windows_sys::Win32::Foundation::GetLastError;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
|
||||
use windows_sys::Win32::Storage::FileSystem::PIPE_ACCESS_DUPLEX;
|
||||
use windows_sys::Win32::System::Pipes::ConnectNamedPipe;
|
||||
use windows_sys::Win32::System::Pipes::CreateNamedPipeW;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_NOWAIT;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_READMODE_BYTE;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_TYPE_BYTE;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_WAIT;
|
||||
|
||||
#[test]
|
||||
fn relative_volume_opens_files_and_rejects_junctions() -> io::Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let real = std::fs::canonicalize(temp.path())?;
|
||||
let open = |path: &Path, access, disposition, options| {
|
||||
volume_fallback::open_relative_to_volume(&mut nt_path(path)?, access, disposition, options)
|
||||
};
|
||||
let existing = real.join("existing.txt");
|
||||
std::fs::File::from(open(
|
||||
&existing,
|
||||
FILE_WRITE_DATA,
|
||||
FILE_CREATE,
|
||||
FILE_NON_DIRECTORY_FILE,
|
||||
)?)
|
||||
.write_all(b"updated")?;
|
||||
let file = std::fs::File::from(open(
|
||||
&existing,
|
||||
FILE_GENERIC_READ,
|
||||
FILE_OPEN,
|
||||
FILE_NON_DIRECTORY_FILE,
|
||||
)?);
|
||||
assert_eq!(file.metadata()?.len(), 7);
|
||||
|
||||
let drive_root = existing.ancestors().last().expect("absolute path root");
|
||||
let root = std::fs::File::from(open(
|
||||
drive_root,
|
||||
FILE_READ_ATTRIBUTES,
|
||||
FILE_OPEN,
|
||||
FILE_DIRECTORY_FILE,
|
||||
)?);
|
||||
assert!(root.metadata()?.is_dir());
|
||||
|
||||
let junction_parent = tempfile::tempdir()?;
|
||||
let junction = junction_parent.path().join("junction");
|
||||
let result = std::process::Command::new("cmd")
|
||||
.args(["/C", "mklink", "/J"])
|
||||
.arg(&junction)
|
||||
.arg(&real)
|
||||
.output()?;
|
||||
assert!(
|
||||
result.status.success(),
|
||||
"{}",
|
||||
String::from_utf8_lossy(&result.stderr)
|
||||
);
|
||||
for (path, disposition, options) in [
|
||||
(junction.clone(), FILE_OPEN, FILE_DIRECTORY_FILE),
|
||||
(
|
||||
junction.join("existing.txt"),
|
||||
FILE_OPEN,
|
||||
FILE_NON_DIRECTORY_FILE,
|
||||
),
|
||||
(
|
||||
junction.join("blocked.txt"),
|
||||
FILE_CREATE,
|
||||
FILE_NON_DIRECTORY_FILE,
|
||||
),
|
||||
] {
|
||||
let error = open(&path, FILE_READ_ATTRIBUTES, disposition, options)
|
||||
.expect_err("a junction must never be traversed");
|
||||
assert_eq!(error.to_string(), "path contains a reparse point");
|
||||
}
|
||||
assert!(!real.join("blocked.txt").exists());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn volume_root_validation_rejects_devices_and_subdirectories() {
|
||||
for (path, expected) in [
|
||||
(r"\Device\HarddiskVolume1\", true),
|
||||
(r"\Device\HarddiskVolume123\", true),
|
||||
(r"\Device\HarddiskVolume\", false),
|
||||
(r"\Device\HarddiskVolume1", false),
|
||||
(r"\Device\HarddiskVolume1\workspace\", false),
|
||||
(r"\Device\HarddiskVolume1x\", false),
|
||||
(r"\Device\NamedPipe\", false),
|
||||
(r"\Device\Mup\", false),
|
||||
(r"\Device\CdRom0\", false),
|
||||
] {
|
||||
let path = OsStr::new(path).encode_wide().collect::<Vec<_>>();
|
||||
assert_eq!(volume_fallback::is_local_volume_root(&path), expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn native_open_rejects_named_pipes_before_connecting() {
|
||||
@@ -20,7 +107,7 @@ fn native_open_rejects_named_pipes_before_connecting() {
|
||||
CreateNamedPipeW(
|
||||
wide_path.as_ptr(),
|
||||
PIPE_ACCESS_DUPLEX,
|
||||
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
|
||||
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_NOWAIT,
|
||||
/*nmaxinstances*/ 1,
|
||||
/*noutbuffersize*/ 1,
|
||||
/*ninbuffersize*/ 1,
|
||||
@@ -31,6 +118,14 @@ fn native_open_rejects_named_pipes_before_connecting() {
|
||||
assert_ne!(pipe, INVALID_HANDLE_VALUE);
|
||||
let _pipe = unsafe { OwnedHandle::from_raw_handle(pipe) };
|
||||
|
||||
// A DOS drive alias could target a pipe; the root open must reject a pipe
|
||||
// before obtaining a client connection, even before final-path validation.
|
||||
let mut pipe_root: Vec<_> = format!(r"\Device\NamedPipe\{pipe_name}\")
|
||||
.encode_utf16()
|
||||
.chain(std::iter::once(0))
|
||||
.collect();
|
||||
assert!(volume_fallback::open_volume_root(&mut pipe_root).is_err());
|
||||
|
||||
let error = open_handle(
|
||||
&client_path,
|
||||
FILE_GENERIC_READ | FILE_GENERIC_WRITE,
|
||||
@@ -40,4 +135,6 @@ fn native_open_rejects_named_pipes_before_connecting() {
|
||||
.expect_err("strict native open should reject named pipes");
|
||||
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
|
||||
assert_eq!(error.to_string(), "path contains a reparse point");
|
||||
assert_eq!(unsafe { ConnectNamedPipe(pipe, ptr::null_mut()) }, 0);
|
||||
assert_eq!(unsafe { GetLastError() }, ERROR_PIPE_LISTENING);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
//! Retry drive-letter opens from a verified physical volume root when strict NT opens reject
|
||||
//! the DOS alias. The root must be a local directory; all path components below it stay no-follow.
|
||||
|
||||
use super::*;
|
||||
use std::os::windows::fs::MetadataExt;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_TRAVERSE;
|
||||
use windows_sys::Win32::Storage::FileSystem::GetFinalPathNameByHandleW;
|
||||
use windows_sys::Win32::Storage::FileSystem::VOLUME_NAME_NT;
|
||||
|
||||
const FILE_OPEN_REPARSE_POINT: u32 = 0x0020_0000;
|
||||
const DOS_DRIVE_ROOT_LEN: usize = r"\??\C:\".len();
|
||||
|
||||
pub(super) fn open_relative_to_volume(
|
||||
name: &mut [u16],
|
||||
desired_access: u32,
|
||||
create_disposition: u32,
|
||||
create_options: u32,
|
||||
) -> io::Result<OwnedHandle> {
|
||||
let mut root_name = name[..DOS_DRIVE_ROOT_LEN].to_vec();
|
||||
root_name.push(0);
|
||||
let root = open_volume_root(&mut root_name)?;
|
||||
open_nt_handle(
|
||||
&mut name[DOS_DRIVE_ROOT_LEN..],
|
||||
Some(&root),
|
||||
desired_access,
|
||||
create_disposition,
|
||||
create_options,
|
||||
OBJ_DONT_REPARSE,
|
||||
)?
|
||||
.ok_or_else(reparse_error)
|
||||
}
|
||||
|
||||
pub(super) fn open_volume_root(name: &mut [u16]) -> io::Result<OwnedHandle> {
|
||||
let handle = open_nt_handle(
|
||||
name,
|
||||
/*root_directory*/ None,
|
||||
FILE_READ_ATTRIBUTES | FILE_TRAVERSE,
|
||||
FILE_OPEN,
|
||||
FILE_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT,
|
||||
/*attributes*/ 0,
|
||||
)?
|
||||
.ok_or_else(reparse_error)?;
|
||||
let root = std::fs::File::from(handle);
|
||||
let metadata = root.metadata()?;
|
||||
if !metadata.is_dir() || metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 {
|
||||
return Err(reparse_error());
|
||||
}
|
||||
|
||||
let mut name = [0u16; 128];
|
||||
let length = unsafe {
|
||||
GetFinalPathNameByHandleW(
|
||||
root.as_raw_handle(),
|
||||
name.as_mut_ptr(),
|
||||
name.len() as u32,
|
||||
VOLUME_NAME_NT,
|
||||
)
|
||||
} as usize;
|
||||
if length == 0 {
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
if length >= name.len() || !is_local_volume_root(&name[..length]) {
|
||||
return Err(reparse_error());
|
||||
}
|
||||
Ok(root.into())
|
||||
}
|
||||
|
||||
pub(super) fn is_local_volume_root(name: &[u16]) -> bool {
|
||||
String::from_utf16(name).is_ok_and(|name| {
|
||||
name.strip_prefix(r"\Device\HarddiskVolume")
|
||||
.and_then(|suffix| suffix.strip_suffix('\\'))
|
||||
.is_some_and(|number| !number.is_empty() && number.bytes().all(|b| b.is_ascii_digit()))
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user