Zebediah Figura : server: Check duplicated handle access against the calling thread token and target process token.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: fa1b0fcf6cde6e1f7f5c9782457eb8d7f7b9eff3
URL:    https://source.winehq.org/git/wine.git/?a=commit;h=fa1b0fcf6cde6e1f7f5c9782457eb8d7f7b9eff3

Author: Zebediah Figura <z.figura12 at gmail.com>
Date:   Wed Sep 23 23:44:56 2020 -0500

server: Check duplicated handle access against the calling thread token and target process token.

Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>

---

 dlls/advapi32/tests/security.c |  6 +++---
 server/handle.c                | 13 ++++++++++---
 server/security.h              |  2 +-
 server/token.c                 |  6 ++++--
 server/winstation.c            |  4 ++--
 5 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index c9318bef92..eaaa29866b 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -7704,8 +7704,8 @@ static void test_duplicate_handle_access(void)
 
     SetLastError(0xdeadbeef);
     ret = DuplicateHandle(GetCurrentProcess(), sync_event, pi.hProcess, &event2, EVENT_MODIFY_STATE, FALSE, 0);
-    todo_wine ok(!ret, "expected failure\n");
-    todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError());
+    ok(!ret, "expected failure\n");
+    ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError());
 
     ret = WaitForSingleObject(pi.hProcess, 1000);
     ok(!ret, "wait failed\n");
@@ -7736,7 +7736,7 @@ static void test_duplicate_handle_access_child(void)
     ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError());
 
     ret = DuplicateHandle(process, event, process, &event2, EVENT_MODIFY_STATE, FALSE, 0);
-    todo_wine ok(ret, "got error %u\n", GetLastError());
+    ok(ret, "got error %u\n", GetLastError());
 
     SetLastError(0xdeadbeef);
     ret = DuplicateHandle(process, event, GetCurrentProcess(), &event2, EVENT_MODIFY_STATE, FALSE, 0);
diff --git a/server/handle.c b/server/handle.c
index 90f9ea63d6..d4df33f473 100644
--- a/server/handle.c
+++ b/server/handle.c
@@ -286,7 +286,7 @@ obj_handle_t alloc_handle( struct process *process, void *ptr, unsigned int acce
 {
     struct object *obj = ptr;
     access = obj->ops->map_access( obj, access ) & ~RESERVED_ALL;
-    if (access && !check_object_access( obj, &access )) return 0;
+    if (access && !check_object_access( NULL, obj, &access )) return 0;
     return alloc_handle_entry( process, ptr, access, attr );
 }
 
@@ -308,7 +308,7 @@ static obj_handle_t alloc_global_handle_no_access_check( void *obj, unsigned int
 /* return the handle, or 0 on error */
 static obj_handle_t alloc_global_handle( void *obj, unsigned int access )
 {
-    if (access && !check_object_access( obj, &access )) return 0;
+    if (access && !check_object_access( NULL, obj, &access )) return 0;
     return alloc_global_handle_no_access_check( obj, access );
 }
 
@@ -558,10 +558,17 @@ obj_handle_t duplicate_handle( struct process *src, obj_handle_t src_handle, str
     /* asking for the more access rights than src_access? */
     if (access & ~src_access)
     {
+        if ((current->token && !check_object_access( current->token, obj, &access )) ||
+            !check_object_access( dst->token, obj, &access ))
+        {
+            release_object( obj );
+            return 0;
+        }
+
         if (options & DUP_HANDLE_MAKE_GLOBAL)
             res = alloc_global_handle( obj, access );
         else
-            res = alloc_handle( dst, obj, access, attr );
+            res = alloc_handle_no_access_check( dst, obj, access, attr );
     }
     else
     {
diff --git a/server/security.h b/server/security.h
index bece7f5404..08bdb8de80 100644
--- a/server/security.h
+++ b/server/security.h
@@ -86,7 +86,7 @@ static inline int security_equal_sid( const SID *sid1, const SID *sid2 )
 
 extern void security_set_thread_token( struct thread *thread, obj_handle_t handle );
 extern const SID *security_unix_uid_to_sid( uid_t uid );
-extern int check_object_access( struct object *obj, unsigned int *access );
+extern int check_object_access( struct token *token, struct object *obj, unsigned int *access );
 
 static inline int thread_single_check_privilege( struct thread *thread, const LUID *priv)
 {
diff --git a/server/token.c b/server/token.c
index 8a106b7d34..26d9708f2c 100644
--- a/server/token.c
+++ b/server/token.c
@@ -1208,13 +1208,15 @@ const SID *token_get_primary_group( struct token *token )
     return token->primary_group;
 }
 
-int check_object_access(struct object *obj, unsigned int *access)
+int check_object_access(struct token *token, struct object *obj, unsigned int *access)
 {
     GENERIC_MAPPING mapping;
-    struct token *token = current->token ? current->token : current->process->token;
     unsigned int status;
     int res;
 
+    if (!token)
+        token = current->token ? current->token : current->process->token;
+
     mapping.GenericAll = obj->ops->map_access( obj, GENERIC_ALL );
 
     if (!obj->sd)
diff --git a/server/winstation.c b/server/winstation.c
index b014290854..c9c85e50ff 100644
--- a/server/winstation.c
+++ b/server/winstation.c
@@ -717,7 +717,7 @@ DECL_HANDLER(enum_winstation)
     {
         unsigned int access = WINSTA_ENUMERATE;
         if (req->index > index++) continue;
-        if (!check_object_access( &winsta->obj, &access )) continue;
+        if (!check_object_access( NULL, &winsta->obj, &access )) continue;
         clear_error();
         reply->next = index;
         if ((name = get_object_name( &winsta->obj, &len )))
@@ -746,7 +746,7 @@ DECL_HANDLER(enum_desktop)
         unsigned int access = DESKTOP_ENUMERATE;
         if (req->index > index++) continue;
         if (!desktop->obj.name) continue;
-        if (!check_object_access( &desktop->obj, &access )) continue;
+        if (!check_object_access( NULL, &desktop->obj, &access )) continue;
         if ((name = get_object_name( &desktop->obj, &len )))
             set_reply_data( name, min( len, get_reply_max_size() ));
         release_object( winstation );