server: Check object's security when creating handles.
Robert Shearman
rob at codeweavers.com
Wed Feb 21 07:59:59 CST 2007
Don't check object's security when duplicating a handle of the same or
lower access rights.
(Based on a patch by Vitaliy Margolen.)
---
dlls/advapi32/tests/security.c | 2 +
server/handle.c | 66
+++++++++++++++++++++++++++++++---------
server/security.h | 1 +
server/token.c | 32 +++++++++++++++++++
4 files changed, 85 insertions(+), 16 deletions(-)
Changes from Vitaliy's original patch:
- Added a condition for MAXIMUM_ALLOWED in check_object_access when no
sd is available.
- Removed the calling of get_handle from within duplicate_handle by
adding _no_access_check of the alloc_handle functions.
-------------- next part --------------
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index ef342b5..201c649 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -857,7 +857,7 @@ static void test_token_attr(void)
BYTE buf[1024];
DWORD bufsize = sizeof(buf);
ret = GetTokenInformation(Token, TokenUser,(void*)buf, bufsize, &bufsize);
- todo_wine ok(ret, "GetTokenInformation failed with error %d\n", GetLastError());
+ ok(ret, "GetTokenInformation failed with error %d\n", GetLastError());
CloseHandle(Token);
}
diff --git a/server/handle.c b/server/handle.c
index 4b52280..852b3f5 100644
--- a/server/handle.c
+++ b/server/handle.c
@@ -36,6 +36,7 @@ #include "winternl.h"
#include "handle.h"
#include "process.h"
#include "thread.h"
+#include "security.h"
#include "request.h"
struct handle_entry
@@ -222,11 +223,10 @@ static obj_handle_t alloc_entry( struct
/* allocate a handle for an object, incrementing its refcount */
/* return the handle, or 0 on error */
-obj_handle_t alloc_handle( struct process *process, void *ptr, unsigned int access, unsigned int attr )
+static obj_handle_t alloc_handle_no_access_check( struct process *process, void *ptr, unsigned int access, unsigned int attr )
{
struct object *obj = ptr;
- access = obj->ops->map_access( obj, access );
access &= ~RESERVED_ALL;
if (attr & OBJ_INHERIT) access |= RESERVED_INHERIT;
if (!process->handles)
@@ -237,9 +237,20 @@ obj_handle_t alloc_handle( struct proces
return alloc_entry( process->handles, obj, access );
}
+/* allocate a handle for an object, checking the dacl allows the process to */
+/* access it and incrementing its refcount */
+/* return the handle, or 0 on error */
+obj_handle_t alloc_handle( struct process *process, void *ptr, unsigned int access, unsigned int attr )
+{
+ struct object *obj = ptr;
+ access = obj->ops->map_access( obj, access );
+ if (access && !check_object_access( obj, &access )) return 0;
+ return alloc_handle_no_access_check( process, ptr, access, attr );
+}
+
/* allocate a global handle for an object, incrementing its refcount */
/* return the handle, or 0 on error */
-static obj_handle_t alloc_global_handle( void *obj, unsigned int access )
+static obj_handle_t alloc_global_handle_no_access_check( void *obj, unsigned int access )
{
if (!global_table)
{
@@ -250,6 +261,15 @@ static obj_handle_t alloc_global_handle(
return handle_local_to_global( alloc_entry( global_table, obj, access ));
}
+/* allocate a global handle for an object, checking the dacl allows the */
+/* process to access it and incrementing its refcount and incrementing its refcount */
+/* 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;
+ return alloc_global_handle_no_access_check( obj, access );
+}
+
/* return a handle entry, or NULL if the handle is invalid */
static struct handle_entry *get_handle( struct process *process, obj_handle_t handle )
{
@@ -449,25 +469,41 @@ obj_handle_t duplicate_handle( struct pr
unsigned int access, unsigned int attr, unsigned int options )
{
obj_handle_t res;
+ struct handle_entry *entry;
+ unsigned int src_access;
struct object *obj = get_handle_obj( src, src_handle, 0, NULL );
if (!obj) return 0;
+ if ((entry = get_handle( src, src_handle )))
+ src_access = entry->access;
+ else /* pseudo-handle, give it full access */
+ {
+ src_access = obj->ops->map_access( obj, GENERIC_ALL );
+ clear_error();
+ }
+ src_access &= ~RESERVED_ALL;
+
if (options & DUP_HANDLE_SAME_ACCESS)
+ access = src_access;
+ else
+ access = obj->ops->map_access( obj, access ) & ~RESERVED_ALL;
+
+ /* asking for the more access rights than src_access? */
+ if (access & ~src_access)
{
- struct handle_entry *entry = get_handle( src, src_handle );
- if (entry)
- access = entry->access;
- else /* pseudo-handle, give it full access */
- {
- access = obj->ops->map_access( obj, GENERIC_ALL );
- clear_error();
- }
+ if (options & DUP_HANDLE_MAKE_GLOBAL)
+ res = alloc_global_handle( obj, access );
+ else
+ res = alloc_handle( dst, obj, access, attr );
}
- access &= ~RESERVED_ALL;
- if (options & DUP_HANDLE_MAKE_GLOBAL)
- res = alloc_global_handle( obj, access );
else
- res = alloc_handle( dst, obj, access, attr );
+ {
+ if (options & DUP_HANDLE_MAKE_GLOBAL)
+ res = alloc_global_handle_no_access_check( obj, access );
+ else
+ res = alloc_handle_no_access_check( dst, obj, access, attr );
+ }
+
release_object( obj );
return res;
}
diff --git a/server/security.h b/server/security.h
index 191688d..e2a3c0b 100644
--- a/server/security.h
+++ b/server/security.h
@@ -57,3 +57,4 @@ static inline int thread_single_check_pr
return token_check_privileges( token, TRUE, &privs, 1, NULL );
}
+extern int check_object_access(struct object *obj, unsigned int *access);
diff --git a/server/token.c b/server/token.c
index f49b355..fe0ae3d 100644
--- a/server/token.c
+++ b/server/token.c
@@ -1005,6 +1005,38 @@ static void set_object_sd( struct object
obj->sd = pnew_sd;
}
+int check_object_access(struct object *obj, unsigned int *access)
+{
+ GENERIC_MAPPING mapping;
+ struct token *token = current->token ? current->token : current->process->token;
+ LUID_AND_ATTRIBUTES priv;
+ unsigned int status, priv_count = 1;
+ int res;
+
+ mapping.GenericAll = obj->ops->map_access( obj, GENERIC_ALL );
+
+ if (!obj->sd)
+ {
+ if (*access & MAXIMUM_ALLOWED)
+ *access = mapping.GenericAll;
+ return TRUE;
+ }
+
+ mapping.GenericRead = obj->ops->map_access( obj, GENERIC_READ );
+ mapping.GenericWrite = obj->ops->map_access( obj, GENERIC_WRITE );
+ mapping.GenericExecute = obj->ops->map_access( obj, GENERIC_EXECUTE );
+
+ res = token_access_check( token, obj->sd, *access, &priv, &priv_count,
+ &mapping, access, &status ) == STATUS_SUCCESS &&
+ status == STATUS_SUCCESS;
+ if (!res)
+ {
+ if (debug_level) fprintf( stderr, "access check failed\n" );
+ set_error( STATUS_ACCESS_DENIED );
+ }
+ return res;
+}
+
/* open a security token */
DECL_HANDLER(open_token)
{
More information about the wine-patches
mailing list