diff --git a/codex-rs/exec-server/src/no_follow/windows.rs b/codex-rs/exec-server/src/no_follow/windows.rs index 0a66882678..3492979b5a 100644 --- a/codex-rs/exec-server/src/no_follow/windows.rs +++ b/codex-rs/exec-server/src/no_follow/windows.rs @@ -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 { - let mut path = nt_path(path)?; - let name_length = u16::try_from((path.len() - 1) * size_of::()) - .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "filesystem path is too long"))?; - let maximum_length = u16::try_from(path.len() * size_of::()) + 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> { + 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::() as u16, MaximumLength: maximum_length, Buffer: path.as_mut_ptr(), }; @@ -143,9 +184,9 @@ fn open_handle( }; let object_attributes = ObjectAttributes { length: size_of::() 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 { diff --git a/codex-rs/exec-server/src/no_follow/windows_tests.rs b/codex-rs/exec-server/src/no_follow/windows_tests.rs index a07cf74b4b..6274990971 100644 --- a/codex-rs/exec-server/src/no_follow/windows_tests.rs +++ b/codex-rs/exec-server/src/no_follow/windows_tests.rs @@ -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::>(); + 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); } diff --git a/codex-rs/exec-server/src/no_follow/windows_volume_fallback.rs b/codex-rs/exec-server/src/no_follow/windows_volume_fallback.rs new file mode 100644 index 0000000000..aee889f1c3 --- /dev/null +++ b/codex-rs/exec-server/src/no_follow/windows_volume_fallback.rs @@ -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 { + 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 { + 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())) + }) +}