Skip to content

Fix three Win32-only defects in proc_open descriptor handling - #23412

Closed
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/proc-open-win32-hygiene
Closed

iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/proc-open-win32-hygiene

Conversation

@iliaal

@iliaal iliaal commented Aug 22, 2026

Copy link
Copy Markdown
Member

Fix three Win32-only defects in proc_open: init_process_info() memset the pointer parameter instead of the PROCESS_INFORMATION structure it points at; find_comspec_nt()'s cleanup dereferenced *comspec before the caller assigned it on the SearchPathW() failure path; and set_proc_descriptor_to_blackhole() tested CreateFileA() against NULL instead of INVALID_HANDLE_VALUE.

init_process_info() memset the pointer parameter instead of the
PROCESS_INFORMATION structure it points at, leaving the struct
uninitialized before CreateProcessW(). Zero it through the pointer.

find_comspec_nt() dereferences *comspec in its cleanup while the
caller only assigns it on success, so a failed SearchPathW() read an
indeterminate value. Initialize the caller's variable to NULL.

set_proc_descriptor_to_blackhole() tested CreateFileA() against NULL,
but CreateFileA() signals failure with INVALID_HANDLE_VALUE, so a
failed open went undetected and an invalid handle was inherited by
the child. Test against INVALID_HANDLE_VALUE.
Comment thread ext/standard/proc_open.c

static void init_process_info(PROCESS_INFORMATION *pi)
{
memset(&pi, 0, sizeof(pi));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes

@iliaal iliaal closed this in 8d0d630 Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants