Skip to content

W
wine-winehq
Commit ddd161f3 authored by Paul Gofman's avatar Paul Gofman Committed by Alexandre Julliard
Browse files
Options

ntdll: Validate job handles at process creation.

Signed-off-by: 's avatarPaul Gofman <pgofman@codeweavers.com> Signed-off-by: 's avatarAlexandre Julliard <julliard@winehq.org>
parent 0f4bc322
Hide whitespace changes
Inline Side-by-side
Showing with 82 additions and 5 deletions
dlls/kernel32/tests/process.c
View file @ ddd161f3
...@@ -4540,6 +4540,9 @@ static void test_nested_jobs(void) ...@@ -4540,6 +4540,9 @@ static void test_nested_jobs(void)
static void test_job_list_attribute(void) static void test_job_list_attribute(void)
{ {
PPROC_THREAD_ATTRIBUTE_LIST attrs; PPROC_THREAD_ATTRIBUTE_LIST attrs;
char buffer[MAX_PATH + 19];
PROCESS_INFORMATION pi;
STARTUPINFOEXA si;
HANDLE jobs[2]; HANDLE jobs[2];
SIZE_T size; SIZE_T size;
BOOL ret; BOOL ret;
...@@ -4586,6 +4589,30 @@ static void test_job_list_attribute(void) ...@@ -4586,6 +4589,30 @@ static void test_job_list_attribute(void)
sizeof(*jobs) * 2, NULL, NULL); sizeof(*jobs) * 2, NULL, NULL);
ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError()); ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError());
ret = pInitializeProcThreadAttributeList(attrs, 1, 0, &size);
ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError());
ret = pUpdateProcThreadAttribute(attrs, 0, PROC_THREAD_ATTRIBUTE_JOB_LIST, jobs,
sizeof(*jobs), NULL, NULL);
memset(&si, 0, sizeof(si));
si.StartupInfo.cb = sizeof(si);
si.lpAttributeList = attrs;
sprintf(buffer, "\"%s\" process wait", selfname);
ret = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, EXTENDED_STARTUPINFO_PRESENT, NULL, NULL,
(STARTUPINFOA *)&si, &pi);
ok(!ret && GetLastError() == ERROR_INVALID_HANDLE, "Got unexpected ret %#x, GetLastError() %u.\n",
ret, GetLastError());
ret = pInitializeProcThreadAttributeList(attrs, 1, 0, &size);
ok(ret, "Got unexpected ret %#x, GetLastError() %u.\n", ret, GetLastError());
ret = pUpdateProcThreadAttribute(attrs, 0, PROC_THREAD_ATTRIBUTE_JOB_LIST, jobs + 1,
sizeof(*jobs), NULL, NULL);
ret = CreateProcessA(NULL, buffer, NULL, NULL, FALSE, EXTENDED_STARTUPINFO_PRESENT, NULL, NULL,
(STARTUPINFOA *)&si, &pi);
ok(!ret && GetLastError() == ERROR_INVALID_HANDLE, "Got unexpected ret %#x, GetLastError() %u.\n",
ret, GetLastError());
pDeleteProcThreadAttributeList(attrs); pDeleteProcThreadAttributeList(attrs);
heap_free(attrs); heap_free(attrs);
} }
......
dlls/ntdll/unix/process.c
View file @ ddd161f3
...@@ -628,9 +628,9 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ ...@@ -628,9 +628,9 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
UNICODE_STRING redir, path = {0}; UNICODE_STRING redir, path = {0};
OBJECT_ATTRIBUTES attr, empty_attr = { sizeof(empty_attr) }; OBJECT_ATTRIBUTES attr, empty_attr = { sizeof(empty_attr) };
SIZE_T i, attr_count = (ps_attr->TotalLength - sizeof(ps_attr->TotalLength)) / sizeof(PS_ATTRIBUTE); SIZE_T i, attr_count = (ps_attr->TotalLength - sizeof(ps_attr->TotalLength)) / sizeof(PS_ATTRIBUTE);
const PS_ATTRIBUTE *handles_attr = NULL; const PS_ATTRIBUTE *handles_attr = NULL, *jobs_attr = NULL;
data_size_t handles_size; data_size_t handles_size, jobs_size;
obj_handle_t *handles; obj_handle_t *handles, *jobs;
for (i = 0; i < attr_count; i++) for (i = 0; i < attr_count; i++)
{ {
...@@ -653,6 +653,9 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ ...@@ -653,6 +653,9 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
if (process_flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES) if (process_flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES)
handles_attr = &ps_attr->Attributes[i]; handles_attr = &ps_attr->Attributes[i];
break; break;
case PS_ATTRIBUTE_JOB_LIST:
jobs_attr = &ps_attr->Attributes[i];
break;
default: default:
if (ps_attr->Attributes[i].Attribute & PS_ATTRIBUTE_INPUT) if (ps_attr->Attributes[i].Attribute & PS_ATTRIBUTE_INPUT)
FIXME( "unhandled input attribute %lx\n", ps_attr->Attributes[i].Attribute ); FIXME( "unhandled input attribute %lx\n", ps_attr->Attributes[i].Attribute );
...@@ -690,6 +693,13 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ ...@@ -690,6 +693,13 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
goto done; goto done;
} }
if ((status = alloc_handle_list( jobs_attr, &jobs, &jobs_size )))
{
free( objattr );
free( handles );
goto done;
}
/* create the socket for the new process */ /* create the socket for the new process */
if (socketpair( PF_UNIX, SOCK_STREAM, 0, socketfd ) == -1) if (socketpair( PF_UNIX, SOCK_STREAM, 0, socketfd ) == -1)
...@@ -697,6 +707,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ ...@@ -697,6 +707,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
status = STATUS_TOO_MANY_OPENED_FILES; status = STATUS_TOO_MANY_OPENED_FILES;
free( objattr ); free( objattr );
free( handles ); free( handles );
free( jobs );
goto done; goto done;
} }
#ifdef SO_PASSCRED #ifdef SO_PASSCRED
...@@ -722,8 +733,10 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ ...@@ -722,8 +733,10 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
req->access = process_access; req->access = process_access;
req->info_size = startup_info_size; req->info_size = startup_info_size;
req->handles_size = handles_size; req->handles_size = handles_size;
req->jobs_size = jobs_size;
wine_server_add_data( req, objattr, attr_len ); wine_server_add_data( req, objattr, attr_len );
wine_server_add_data( req, handles, handles_size ); wine_server_add_data( req, handles, handles_size );
wine_server_add_data( req, jobs, jobs_size );
wine_server_add_data( req, startup_info, startup_info_size ); wine_server_add_data( req, startup_info, startup_info_size );
wine_server_add_data( req, params->Environment, env_size ); wine_server_add_data( req, params->Environment, env_size );
if (!(status = wine_server_call( req ))) if (!(status = wine_server_call( req )))
...@@ -736,6 +749,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ ...@@ -736,6 +749,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
SERVER_END_REQ; SERVER_END_REQ;
free( objattr ); free( objattr );
free( handles ); free( handles );
free( jobs );
if (status) if (status)
{ {
......
include/wine/server_protocol.h
View file @ ddd161f3
...@@ -841,10 +841,13 @@ struct new_process_request ...@@ -841,10 +841,13 @@ struct new_process_request
char __pad_38[2]; char __pad_38[2];
data_size_t info_size; data_size_t info_size;
data_size_t handles_size; data_size_t handles_size;
data_size_t jobs_size;
/* VARARG(objattr,object_attributes); */ /* VARARG(objattr,object_attributes); */
/* VARARG(handles,uints,handles_size); */ /* VARARG(handles,uints,handles_size); */
/* VARARG(jobs,uints,jobs_size); */
/* VARARG(info,startup_info,info_size); */ /* VARARG(info,startup_info,info_size); */
/* VARARG(env,unicode_str); */ /* VARARG(env,unicode_str); */
char __pad_52[4];
}; };
struct new_process_reply struct new_process_reply
{ {
...@@ -6244,7 +6247,7 @@ union generic_reply ...@@ -6244,7 +6247,7 @@ union generic_reply
/* ### protocol_version begin ### */ /* ### protocol_version begin ### */
#define SERVER_PROTOCOL_VERSION 701 #define SERVER_PROTOCOL_VERSION 702
/* ### protocol_version end ### */ /* ### protocol_version end ### */
......
server/process.c
View file @ ddd161f3
...@@ -1106,6 +1106,8 @@ DECL_HANDLER(new_process) ...@@ -1106,6 +1106,8 @@ DECL_HANDLER(new_process)
struct thread *parent_thread = current; struct thread *parent_thread = current;
int socket_fd = thread_get_inflight_fd( current, req->socket_fd ); int socket_fd = thread_get_inflight_fd( current, req->socket_fd );
const obj_handle_t *handles = NULL; const obj_handle_t *handles = NULL;
const obj_handle_t *job_handles = NULL;
unsigned int i, job_handle_count;
struct job *job; struct job *job;
if (socket_fd == -1) if (socket_fd == -1)
...@@ -1178,6 +1180,31 @@ DECL_HANDLER(new_process) ...@@ -1178,6 +1180,31 @@ DECL_HANDLER(new_process)
info_ptr = (const char *)info_ptr + req->handles_size; info_ptr = (const char *)info_ptr + req->handles_size;
info->data_size -= req->handles_size; info->data_size -= req->handles_size;
} }
if ((req->jobs_size & 3) || req->jobs_size > info->data_size)
{
set_error( STATUS_INVALID_PARAMETER );
close( socket_fd );
goto done;
}
if (req->jobs_size)
{
job_handles = info_ptr;
info_ptr = (const char *)info_ptr + req->jobs_size;
info->data_size -= req->jobs_size;
}
job_handle_count = req->jobs_size / sizeof(*handles);
for (i = 0; i < job_handle_count; ++i)
{
if (!(job = get_job_obj( current->process, job_handles[i], JOB_OBJECT_ASSIGN_PROCESS )))
{
close( socket_fd );
goto done;
}
release_object( job );
}
info->info_size = min( req->info_size, info->data_size ); info->info_size = min( req->info_size, info->data_size );
if (req->info_size < sizeof(*info->data)) if (req->info_size < sizeof(*info->data))
......
server/protocol.def
View file @ ddd161f3
...@@ -854,8 +854,10 @@ typedef struct ...@@ -854,8 +854,10 @@ typedef struct
unsigned short machine; /* architecture that the new process will use */ unsigned short machine; /* architecture that the new process will use */
data_size_t info_size; /* size of startup info */ data_size_t info_size; /* size of startup info */
data_size_t handles_size; /* length of explicit handles list */ data_size_t handles_size; /* length of explicit handles list */
data_size_t jobs_size; /* length of jobs list */
VARARG(objattr,object_attributes); /* object attributes */ VARARG(objattr,object_attributes); /* object attributes */
VARARG(handles,uints,handles_size); /* handles list */ VARARG(handles,uints,handles_size); /* handles list */
VARARG(jobs,uints,jobs_size); /* jobs list */
VARARG(info,startup_info,info_size); /* startup information */ VARARG(info,startup_info,info_size); /* startup information */
VARARG(env,unicode_str); /* environment for new process */ VARARG(env,unicode_str); /* environment for new process */
@REPLY @REPLY
......
server/request.h
View file @ ddd161f3
...@@ -714,7 +714,8 @@ C_ASSERT( FIELD_OFFSET(struct new_process_request, access) == 32 ); ...@@ -714,7 +714,8 @@ C_ASSERT( FIELD_OFFSET(struct new_process_request, access) == 32 );
C_ASSERT( FIELD_OFFSET(struct new_process_request, machine) == 36 ); C_ASSERT( FIELD_OFFSET(struct new_process_request, machine) == 36 );
C_ASSERT( FIELD_OFFSET(struct new_process_request, info_size) == 40 ); C_ASSERT( FIELD_OFFSET(struct new_process_request, info_size) == 40 );
C_ASSERT( FIELD_OFFSET(struct new_process_request, handles_size) == 44 ); C_ASSERT( FIELD_OFFSET(struct new_process_request, handles_size) == 44 );
C_ASSERT( sizeof(struct new_process_request) == 48 ); C_ASSERT( FIELD_OFFSET(struct new_process_request, jobs_size) == 48 );
C_ASSERT( sizeof(struct new_process_request) == 56 );
C_ASSERT( FIELD_OFFSET(struct new_process_reply, info) == 8 ); C_ASSERT( FIELD_OFFSET(struct new_process_reply, info) == 8 );
C_ASSERT( FIELD_OFFSET(struct new_process_reply, pid) == 12 ); C_ASSERT( FIELD_OFFSET(struct new_process_reply, pid) == 12 );
C_ASSERT( FIELD_OFFSET(struct new_process_reply, handle) == 16 ); C_ASSERT( FIELD_OFFSET(struct new_process_reply, handle) == 16 );
......
server/trace.c
View file @ ddd161f3
...@@ -1388,8 +1388,10 @@ static void dump_new_process_request( const struct new_process_request *req ) ...@@ -1388,8 +1388,10 @@ static void dump_new_process_request( const struct new_process_request *req )
fprintf( stderr, ", machine=%04x", req->machine ); fprintf( stderr, ", machine=%04x", req->machine );
fprintf( stderr, ", info_size=%u", req->info_size ); fprintf( stderr, ", info_size=%u", req->info_size );
fprintf( stderr, ", handles_size=%u", req->handles_size ); fprintf( stderr, ", handles_size=%u", req->handles_size );
fprintf( stderr, ", jobs_size=%u", req->jobs_size );
dump_varargs_object_attributes( ", objattr=", cur_size ); dump_varargs_object_attributes( ", objattr=", cur_size );
dump_varargs_uints( ", handles=", min(cur_size,req->handles_size) ); dump_varargs_uints( ", handles=", min(cur_size,req->handles_size) );
dump_varargs_uints( ", jobs=", min(cur_size,req->jobs_size) );
dump_varargs_startup_info( ", info=", min(cur_size,req->info_size) ); dump_varargs_startup_info( ", info=", min(cur_size,req->info_size) );
dump_varargs_unicode_str( ", env=", cur_size ); dump_varargs_unicode_str( ", env=", cur_size );
} }
...@@ -5329,6 +5331,7 @@ static const struct ...@@ -5329,6 +5331,7 @@ static const struct
{ "CANT_OPEN_ANONYMOUS", STATUS_CANT_OPEN_ANONYMOUS }, { "CANT_OPEN_ANONYMOUS", STATUS_CANT_OPEN_ANONYMOUS },
{ "CHILD_MUST_BE_VOLATILE", STATUS_CHILD_MUST_BE_VOLATILE }, { "CHILD_MUST_BE_VOLATILE", STATUS_CHILD_MUST_BE_VOLATILE },
{ "CONNECTION_ABORTED", STATUS_CONNECTION_ABORTED }, { "CONNECTION_ABORTED", STATUS_CONNECTION_ABORTED },
{ "CONNECTION_ACTIVE", STATUS_CONNECTION_ACTIVE },
{ "CONNECTION_REFUSED", STATUS_CONNECTION_REFUSED }, { "CONNECTION_REFUSED", STATUS_CONNECTION_REFUSED },
{ "CONNECTION_RESET", STATUS_CONNECTION_RESET }, { "CONNECTION_RESET", STATUS_CONNECTION_RESET },
{ "DEBUGGER_INACTIVE", STATUS_DEBUGGER_INACTIVE }, { "DEBUGGER_INACTIVE", STATUS_DEBUGGER_INACTIVE },
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment