From fa1b0fcf6cde6e1f7f5c9782457eb8d7f7b9eff3 Mon Sep 17 00:00:00 2001 From: Zebediah Figura Date: Wed, 23 Sep 2020 23:44:56 -0500 Subject: [PATCH] server: Check duplicated handle access against the calling thread token and target process token. Signed-off-by: Zebediah Figura Signed-off-by: Alexandre Julliard --- 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 c9318bef928..eaaa29866bb 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 90f9ea63d62..d4df33f4730 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 bece7f54048..08bdb8de805 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 8a106b7d34f..26d9708f2cd 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 b0142908549..c9c85e50fff 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 );