mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Prevent ConPTY DLL loading from the current directory (#39262)
## Why The Windows PTY implementation probed `conpty.dll` by bare name. Windows could resolve that lookup to a DLL in the process current directory and load it when creating a pseudoconsole. ## What changed Resolve the ConPTY entry points from `kernel32.dll` without probing `conpty.dll`. ## Testing Add a Windows regression test that places a compatible `conpty.dll` in the current directory, creates a ConPTY, and verifies that the DLL was not loaded. GitOrigin-RevId: be78974362a99dce0c18186cf3e7bbd86a8de73c
This commit is contained in:
@@ -24,6 +24,7 @@ winapi = { version = "0.3.9", features = [
|
||||
"handleapi",
|
||||
"jobapi",
|
||||
"jobapi2",
|
||||
"libloaderapi",
|
||||
"minwinbase",
|
||||
"processthreadsapi",
|
||||
"synchapi",
|
||||
|
||||
@@ -85,20 +85,10 @@ shared_library!(Ntdll,
|
||||
) -> NTSTATUS,
|
||||
);
|
||||
|
||||
fn load_conpty() -> ConPtyFuncs {
|
||||
let kernel = ConPtyFuncs::open(Path::new("kernel32.dll")).expect(
|
||||
lazy_static! {
|
||||
static ref CONPTY: ConPtyFuncs = ConPtyFuncs::open(Path::new("kernel32.dll")).expect(
|
||||
"this system does not support conpty. Windows 10 October 2018 or newer is required",
|
||||
);
|
||||
|
||||
if let Ok(sideloaded) = ConPtyFuncs::open(Path::new("conpty.dll")) {
|
||||
sideloaded
|
||||
} else {
|
||||
kernel
|
||||
}
|
||||
}
|
||||
|
||||
lazy_static! {
|
||||
static ref CONPTY: ConPtyFuncs = load_conpty();
|
||||
}
|
||||
|
||||
pub fn conpty_supported() -> bool {
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
#![cfg(windows)]
|
||||
|
||||
use codex_utils_pty::RawConPty;
|
||||
use std::ffi::OsStr;
|
||||
use std::fs;
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use winapi::um::libloaderapi::GetModuleHandleW;
|
||||
|
||||
struct CurrentDirGuard {
|
||||
original: PathBuf,
|
||||
}
|
||||
|
||||
impl CurrentDirGuard {
|
||||
fn enter(path: &Path) -> std::io::Result<Self> {
|
||||
let original = std::env::current_dir()?;
|
||||
std::env::set_current_dir(path)?;
|
||||
Ok(Self { original })
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for CurrentDirGuard {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::env::set_current_dir(&self.original);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn conpty_ignores_dll_in_current_directory() -> anyhow::Result<()> {
|
||||
let temp_dir = std::env::temp_dir().join(format!(
|
||||
"codex-conpty-search-path-{}-{}",
|
||||
std::process::id(),
|
||||
std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)?
|
||||
.as_nanos()
|
||||
));
|
||||
fs::create_dir(&temp_dir)?;
|
||||
|
||||
let system_root =
|
||||
std::env::var_os("SystemRoot").ok_or_else(|| anyhow::anyhow!("SystemRoot is not set"))?;
|
||||
let candidate = temp_dir.join("conpty.dll");
|
||||
// A kernel32 copy exports the ConPTY functions, so the old bare-name probe
|
||||
// would successfully load and retain this repository-controlled file.
|
||||
fs::copy(
|
||||
PathBuf::from(system_root)
|
||||
.join("System32")
|
||||
.join("kernel32.dll"),
|
||||
&candidate,
|
||||
)?;
|
||||
|
||||
{
|
||||
let _current_dir = CurrentDirGuard::enter(&temp_dir)?;
|
||||
drop(RawConPty::new(/*cols*/ 80, /*rows*/ 24)?);
|
||||
|
||||
let conpty_dll = OsStr::new("conpty.dll")
|
||||
.encode_wide()
|
||||
.chain(std::iter::once(0))
|
||||
.collect::<Vec<_>>();
|
||||
let module = unsafe { GetModuleHandleW(conpty_dll.as_ptr()) };
|
||||
assert!(
|
||||
module.is_null(),
|
||||
"ConPTY loaded conpty.dll from the process current directory"
|
||||
);
|
||||
}
|
||||
|
||||
fs::remove_file(candidate)?;
|
||||
fs::remove_dir(temp_dir)?;
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user