Skip to content

Commit 9114f6c

Browse files
zm-oaicopyberry
zm-oai
authored and
copyberry
committed
Fix Windows 10 drive-letter opens for no-follow filesystem operations
## 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
1 parent eb2d3c6 commit 9114f6c

3 files changed

Lines changed: 225 additions & 16 deletions

File tree

‎codex-rs/exec-server/src/no_follow/windows.rs‎

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::ffi::c_void;
44
use std::io;
55
use std::io::Write;
66
use std::mem::size_of;
7+
use std::mem::size_of_val;
78
use std::os::windows::ffi::OsStrExt;
89
use std::os::windows::io::AsRawHandle;
910
use std::os::windows::io::FromRawHandle;
@@ -36,6 +37,9 @@ use windows_sys::Win32::Storage::FileSystem::SetFileInformationByHandle;
3637
use windows_sys::Win32::System::IO::IO_STATUS_BLOCK;
3738
use windows_sys::Win32::System::IO::IO_STATUS_BLOCK_0;
3839

40+
#[path = "windows_volume_fallback.rs"]
41+
mod volume_fallback;
42+
3943
const FILE_DIRECTORY_FILE: u32 = 0x0000_0001;
4044
const FILE_SYNCHRONOUS_IO_NONALERT: u32 = 0x0000_0020;
4145
const FILE_NON_DIRECTORY_FILE: u32 = 0x0000_0040;
@@ -125,13 +129,50 @@ fn open_handle(
125129
create_disposition: u32,
126130
create_options: u32,
127131
) -> io::Result<OwnedHandle> {
128-
let mut path = nt_path(path)?;
129-
let name_length = u16::try_from((path.len() - 1) * size_of::<u16>())
130-
.map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "filesystem path is too long"))?;
131-
let maximum_length = u16::try_from(path.len() * size_of::<u16>())
132+
let mut name = nt_path(path)?;
133+
if let Some(handle) = open_nt_handle(
134+
&mut name,
135+
/*root_directory*/ None,
136+
desired_access,
137+
create_disposition,
138+
create_options,
139+
OBJ_DONT_REPARSE,
140+
)? {
141+
return Ok(handle);
142+
}
143+
144+
// Windows 10 also rejects the DOS drive alias itself. Only for drive-letter
145+
// paths, open its verified volume root and keep strict checks below it.
146+
if matches!(path.components().next(), Some(Component::Prefix(prefix))
147+
if matches!(prefix.kind(), Prefix::Disk(_) | Prefix::VerbatimDisk(_)))
148+
{
149+
return volume_fallback::open_relative_to_volume(
150+
&mut name,
151+
desired_access,
152+
create_disposition,
153+
create_options,
154+
);
155+
}
156+
Err(reparse_error())
157+
}
158+
159+
fn reparse_error() -> io::Error {
160+
io::Error::new(io::ErrorKind::InvalidInput, "path contains a reparse point")
161+
}
162+
163+
/// Returns `None` only for the native reparse status; other errors propagate.
164+
fn open_nt_handle(
165+
path: &mut [u16],
166+
root_directory: Option<&OwnedHandle>,
167+
desired_access: u32,
168+
create_disposition: u32,
169+
create_options: u32,
170+
attributes: u32,
171+
) -> io::Result<Option<OwnedHandle>> {
172+
let maximum_length = u16::try_from(size_of_val(path))
132173
.map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "filesystem path is too long"))?;
133174
let object_name = UNICODE_STRING {
134-
Length: name_length,
175+
Length: maximum_length - size_of::<u16>() as u16,
135176
MaximumLength: maximum_length,
136177
Buffer: path.as_mut_ptr(),
137178
};
@@ -143,9 +184,9 @@ fn open_handle(
143184
};
144185
let object_attributes = ObjectAttributes {
145186
length: size_of::<ObjectAttributes>() as u32,
146-
root_directory: ptr::null_mut(),
187+
root_directory: root_directory.map_or(ptr::null_mut(), AsRawHandle::as_raw_handle),
147188
object_name: &object_name,
148-
attributes: OBJ_CASE_INSENSITIVE | OBJ_DONT_REPARSE,
189+
attributes: OBJ_CASE_INSENSITIVE | attributes,
149190
security_descriptor: ptr::null(),
150191
security_quality_of_service: (&raw const security_quality_of_service).cast(),
151192
};
@@ -169,13 +210,10 @@ fn open_handle(
169210
/*ea_length*/ 0,
170211
)
171212
};
213+
if status == STATUS_REPARSE_POINT_ENCOUNTERED {
214+
return Ok(None);
215+
}
172216
if status < 0 {
173-
if status == STATUS_REPARSE_POINT_ENCOUNTERED {
174-
return Err(io::Error::new(
175-
io::ErrorKind::InvalidInput,
176-
"path contains a reparse point",
177-
));
178-
}
179217
let code = unsafe { RtlNtStatusToDosError(status) };
180218
return Err(io::Error::from_raw_os_error(code as i32));
181219
}
@@ -185,7 +223,7 @@ fn open_handle(
185223
));
186224
}
187225

188-
Ok(unsafe { OwnedHandle::from_raw_handle(handle) })
226+
Ok(Some(unsafe { OwnedHandle::from_raw_handle(handle) }))
189227
}
190228

191229
fn open_entry(path: &Path) -> io::Result<std::fs::File> {

‎codex-rs/exec-server/src/no_follow/windows_tests.rs‎

Lines changed: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,97 @@
11
use super::*;
2+
use pretty_assertions::assert_eq;
3+
use windows_sys::Win32::Foundation::ERROR_PIPE_LISTENING;
4+
use windows_sys::Win32::Foundation::GetLastError;
25
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
36
use windows_sys::Win32::Storage::FileSystem::PIPE_ACCESS_DUPLEX;
7+
use windows_sys::Win32::System::Pipes::ConnectNamedPipe;
48
use windows_sys::Win32::System::Pipes::CreateNamedPipeW;
9+
use windows_sys::Win32::System::Pipes::PIPE_NOWAIT;
510
use windows_sys::Win32::System::Pipes::PIPE_READMODE_BYTE;
611
use windows_sys::Win32::System::Pipes::PIPE_TYPE_BYTE;
7-
use windows_sys::Win32::System::Pipes::PIPE_WAIT;
12+
13+
#[test]
14+
fn relative_volume_opens_files_and_rejects_junctions() -> io::Result<()> {
15+
let temp = tempfile::tempdir()?;
16+
let real = std::fs::canonicalize(temp.path())?;
17+
let open = |path: &Path, access, disposition, options| {
18+
volume_fallback::open_relative_to_volume(&mut nt_path(path)?, access, disposition, options)
19+
};
20+
let existing = real.join("existing.txt");
21+
std::fs::File::from(open(
22+
&existing,
23+
FILE_WRITE_DATA,
24+
FILE_CREATE,
25+
FILE_NON_DIRECTORY_FILE,
26+
)?)
27+
.write_all(b"updated")?;
28+
let file = std::fs::File::from(open(
29+
&existing,
30+
FILE_GENERIC_READ,
31+
FILE_OPEN,
32+
FILE_NON_DIRECTORY_FILE,
33+
)?);
34+
assert_eq!(file.metadata()?.len(), 7);
35+
36+
let drive_root = existing.ancestors().last().expect("absolute path root");
37+
let root = std::fs::File::from(open(
38+
drive_root,
39+
FILE_READ_ATTRIBUTES,
40+
FILE_OPEN,
41+
FILE_DIRECTORY_FILE,
42+
)?);
43+
assert!(root.metadata()?.is_dir());
44+
45+
let junction_parent = tempfile::tempdir()?;
46+
let junction = junction_parent.path().join("junction");
47+
let result = std::process::Command::new("cmd")
48+
.args(["/C", "mklink", "/J"])
49+
.arg(&junction)
50+
.arg(&real)
51+
.output()?;
52+
assert!(
53+
result.status.success(),
54+
"{}",
55+
String::from_utf8_lossy(&result.stderr)
56+
);
57+
for (path, disposition, options) in [
58+
(junction.clone(), FILE_OPEN, FILE_DIRECTORY_FILE),
59+
(
60+
junction.join("existing.txt"),
61+
FILE_OPEN,
62+
FILE_NON_DIRECTORY_FILE,
63+
),
64+
(
65+
junction.join("blocked.txt"),
66+
FILE_CREATE,
67+
FILE_NON_DIRECTORY_FILE,
68+
),
69+
] {
70+
let error = open(&path, FILE_READ_ATTRIBUTES, disposition, options)
71+
.expect_err("a junction must never be traversed");
72+
assert_eq!(error.to_string(), "path contains a reparse point");
73+
}
74+
assert!(!real.join("blocked.txt").exists());
75+
Ok(())
76+
}
77+
78+
#[test]
79+
fn volume_root_validation_rejects_devices_and_subdirectories() {
80+
for (path, expected) in [
81+
(r"\Device\HarddiskVolume1\", true),
82+
(r"\Device\HarddiskVolume123\", true),
83+
(r"\Device\HarddiskVolume\", false),
84+
(r"\Device\HarddiskVolume1", false),
85+
(r"\Device\HarddiskVolume1\workspace\", false),
86+
(r"\Device\HarddiskVolume1x\", false),
87+
(r"\Device\NamedPipe\", false),
88+
(r"\Device\Mup\", false),
89+
(r"\Device\CdRom0\", false),
90+
] {
91+
let path = OsStr::new(path).encode_wide().collect::<Vec<_>>();
92+
assert_eq!(volume_fallback::is_local_volume_root(&path), expected);
93+
}
94+
}
895

996
#[test]
1097
fn native_open_rejects_named_pipes_before_connecting() {
@@ -20,7 +107,7 @@ fn native_open_rejects_named_pipes_before_connecting() {
20107
CreateNamedPipeW(
21108
wide_path.as_ptr(),
22109
PIPE_ACCESS_DUPLEX,
23-
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
110+
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_NOWAIT,
24111
/*nmaxinstances*/ 1,
25112
/*noutbuffersize*/ 1,
26113
/*ninbuffersize*/ 1,
@@ -31,6 +118,14 @@ fn native_open_rejects_named_pipes_before_connecting() {
31118
assert_ne!(pipe, INVALID_HANDLE_VALUE);
32119
let _pipe = unsafe { OwnedHandle::from_raw_handle(pipe) };
33120

121+
// A DOS drive alias could target a pipe; the root open must reject a pipe
122+
// before obtaining a client connection, even before final-path validation.
123+
let mut pipe_root: Vec<_> = format!(r"\Device\NamedPipe\{pipe_name}\")
124+
.encode_utf16()
125+
.chain(std::iter::once(0))
126+
.collect();
127+
assert!(volume_fallback::open_volume_root(&mut pipe_root).is_err());
128+
34129
let error = open_handle(
35130
&client_path,
36131
FILE_GENERIC_READ | FILE_GENERIC_WRITE,
@@ -40,4 +135,6 @@ fn native_open_rejects_named_pipes_before_connecting() {
40135
.expect_err("strict native open should reject named pipes");
41136
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
42137
assert_eq!(error.to_string(), "path contains a reparse point");
138+
assert_eq!(unsafe { ConnectNamedPipe(pipe, ptr::null_mut()) }, 0);
139+
assert_eq!(unsafe { GetLastError() }, ERROR_PIPE_LISTENING);
43140
}

‎codex-rs/exec-server/src/no_follow/windows_volume_fallback.rs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
//! Retry drive-letter opens from a verified physical volume root when strict NT opens reject
2+
//! the DOS alias. The root must be a local directory; all path components below it stay no-follow.
3+
4+
use super::*;
5+
use std::os::windows::fs::MetadataExt;
6+
use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT;
7+
use windows_sys::Win32::Storage::FileSystem::FILE_TRAVERSE;
8+
use windows_sys::Win32::Storage::FileSystem::GetFinalPathNameByHandleW;
9+
use windows_sys::Win32::Storage::FileSystem::VOLUME_NAME_NT;
10+
11+
const FILE_OPEN_REPARSE_POINT: u32 = 0x0020_0000;
12+
const DOS_DRIVE_ROOT_LEN: usize = r"\??\C:\".len();
13+
14+
pub(super) fn open_relative_to_volume(
15+
name: &mut [u16],
16+
desired_access: u32,
17+
create_disposition: u32,
18+
create_options: u32,
19+
) -> io::Result<OwnedHandle> {
20+
let mut root_name = name[..DOS_DRIVE_ROOT_LEN].to_vec();
21+
root_name.push(0);
22+
let root = open_volume_root(&mut root_name)?;
23+
open_nt_handle(
24+
&mut name[DOS_DRIVE_ROOT_LEN..],
25+
Some(&root),
26+
desired_access,
27+
create_disposition,
28+
create_options,
29+
OBJ_DONT_REPARSE,
30+
)?
31+
.ok_or_else(reparse_error)
32+
}
33+
34+
pub(super) fn open_volume_root(name: &mut [u16]) -> io::Result<OwnedHandle> {
35+
let handle = open_nt_handle(
36+
name,
37+
/*root_directory*/ None,
38+
FILE_READ_ATTRIBUTES | FILE_TRAVERSE,
39+
FILE_OPEN,
40+
FILE_DIRECTORY_FILE | FILE_OPEN_REPARSE_POINT,
41+
/*attributes*/ 0,
42+
)?
43+
.ok_or_else(reparse_error)?;
44+
let root = std::fs::File::from(handle);
45+
let metadata = root.metadata()?;
46+
if !metadata.is_dir() || metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0 {
47+
return Err(reparse_error());
48+
}
49+
50+
let mut name = [0u16; 128];
51+
let length = unsafe {
52+
GetFinalPathNameByHandleW(
53+
root.as_raw_handle(),
54+
name.as_mut_ptr(),
55+
name.len() as u32,
56+
VOLUME_NAME_NT,
57+
)
58+
} as usize;
59+
if length == 0 {
60+
return Err(io::Error::last_os_error());
61+
}
62+
if length >= name.len() || !is_local_volume_root(&name[..length]) {
63+
return Err(reparse_error());
64+
}
65+
Ok(root.into())
66+
}
67+
68+
pub(super) fn is_local_volume_root(name: &[u16]) -> bool {
69+
String::from_utf16(name).is_ok_and(|name| {
70+
name.strip_prefix(r"\Device\HarddiskVolume")
71+
.and_then(|suffix| suffix.strip_suffix('\\'))
72+
.is_some_and(|number| !number.is_empty() && number.bytes().all(|b| b.is_ascii_digit()))
73+
})
74+
}

0 commit comments

Comments
 (0)