[PATCH 3/4] ntdll: Validate job handles at process creation.

Paul Gofman pgofman at codeweavers.com
Wed May 19 19:29:03 CDT 2021


Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
---
 dlls/kernel32/tests/process.c | 27 +++++++++++++++++++++++++++
 dlls/ntdll/unix/process.c     | 20 +++++++++++++++++---
 server/process.c              | 27 +++++++++++++++++++++++++++
 server/protocol.def           |  2 ++
 4 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c
index 9acf0aaaaee..44c37c668fc 100644
--- a/dlls/kernel32/tests/process.c
+++ b/dlls/kernel32/tests/process.c
@@ -4540,6 +4540,9 @@ static void test_nested_jobs(void)
 static void test_job_list_attribute(void)
 {
     PPROC_THREAD_ATTRIBUTE_LIST attrs;
+    char buffer[MAX_PATH + 19];
+    PROCESS_INFORMATION pi;
+    STARTUPINFOEXA si;
     HANDLE jobs[2];
     SIZE_T size;
     BOOL ret;
@@ -4586,6 +4589,30 @@ static void test_job_list_attribute(void)
             sizeof(*jobs) * 2, NULL, NULL);
     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);
     heap_free(attrs);
 }
diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c
index 54e9680699e..340fe9cc304 100644
--- a/dlls/ntdll/unix/process.c
+++ b/dlls/ntdll/unix/process.c
@@ -628,9 +628,9 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
     UNICODE_STRING redir, path = {0};
     OBJECT_ATTRIBUTES attr, empty_attr = { sizeof(empty_attr) };
     SIZE_T i, attr_count = (ps_attr->TotalLength - sizeof(ps_attr->TotalLength)) / sizeof(PS_ATTRIBUTE);
-    const PS_ATTRIBUTE *handles_attr = NULL;
-    data_size_t handles_size;
-    obj_handle_t *handles;
+    const PS_ATTRIBUTE *handles_attr = NULL, *jobs_attr = NULL;
+    data_size_t handles_size, jobs_size;
+    obj_handle_t *handles, *jobs;
 
     for (i = 0; i < attr_count; i++)
     {
@@ -653,6 +653,9 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
             if (process_flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES)
                 handles_attr = &ps_attr->Attributes[i];
             break;
+        case PS_ATTRIBUTE_JOB_LIST:
+            jobs_attr = &ps_attr->Attributes[i];
+            break;
         default:
             if (ps_attr->Attributes[i].Attribute & PS_ATTRIBUTE_INPUT)
                 FIXME( "unhandled input attribute %lx\n", ps_attr->Attributes[i].Attribute );
@@ -690,6 +693,13 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
         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 */
 
     if (socketpair( PF_UNIX, SOCK_STREAM, 0, socketfd ) == -1)
@@ -697,6 +707,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
         status = STATUS_TOO_MANY_OPENED_FILES;
         free( objattr );
         free( handles );
+        free( jobs );
         goto done;
     }
 #ifdef SO_PASSCRED
@@ -722,8 +733,10 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
         req->access         = process_access;
         req->info_size      = startup_info_size;
         req->handles_size   = handles_size;
+        req->jobs_size      = jobs_size;
         wine_server_add_data( req, objattr, attr_len );
         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, params->Environment, env_size );
         if (!(status = wine_server_call( req )))
@@ -736,6 +749,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_
     SERVER_END_REQ;
     free( objattr );
     free( handles );
+    free( jobs );
 
     if (status)
     {
diff --git a/server/process.c b/server/process.c
index 32c6fdd269b..35b0f9d20fb 100644
--- a/server/process.c
+++ b/server/process.c
@@ -1106,6 +1106,8 @@ DECL_HANDLER(new_process)
     struct thread *parent_thread = current;
     int socket_fd = thread_get_inflight_fd( current, req->socket_fd );
     const obj_handle_t *handles = NULL;
+    const obj_handle_t *job_handles = NULL;
+    unsigned int i, job_handle_count;
     struct job *job;
 
     if (socket_fd == -1)
@@ -1178,6 +1180,31 @@ DECL_HANDLER(new_process)
         info_ptr = (const char *)info_ptr + 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 );
 
     if (req->info_size < sizeof(*info->data))
diff --git a/server/protocol.def b/server/protocol.def
index 6d8208b128b..4239be834d1 100644
--- a/server/protocol.def
+++ b/server/protocol.def
@@ -854,8 +854,10 @@ typedef struct
     unsigned short machine;      /* architecture that the new process will use */
     data_size_t  info_size;      /* size of startup info */
     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(handles,uints,handles_size);  /* handles list */
+    VARARG(jobs,uints,jobs_size);        /* jobs list */
     VARARG(info,startup_info,info_size); /* startup information */
     VARARG(env,unicode_str);     /* environment for new process */
 @REPLY
-- 
2.31.1




More information about the wine-devel mailing list