Joris van der Wel : server: Setting a security descriptor should not replace an existing owner or group with a default , if only a DACL is being set.
Module: wine Branch: master Commit: 0a4c7860f82074f7c81d6378075e63848b00b98e URL: http://source.winehq.org/git/wine.git/?a=commit;h=0a4c7860f82074f7c81d637807...
Author: Joris van der Wel joris@jorisvanderwel.com Date: Mon Jul 7 22:12:52 2014 +0200
server: Setting a security descriptor should not replace an existing owner or group with a default, if only a DACL is being set.
---
dlls/advapi32/tests/security.c | 60 ++++++++++++++++++++++++++++++++++++++++++ server/object.c | 22 +++++++++++++--- 2 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index f3ccc8e..b1dc3fe 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -234,6 +234,42 @@ static SECURITY_DESCRIPTOR* test_get_security_descriptor(HANDLE handle, int line return sd; }
+static void test_owner_equal(HANDLE Handle, PSID expected, int line) +{ + BOOL res; + SECURITY_DESCRIPTOR *queriedSD = NULL; + PSID owner; + BOOL owner_defaulted; + + queriedSD = test_get_security_descriptor( Handle, line ); + + res = GetSecurityDescriptorOwner(queriedSD, &owner, &owner_defaulted); + ok_(__FILE__, line)(res, "GetSecurityDescriptorOwner failed with error %d\n", GetLastError()); + + ok_(__FILE__, line)(EqualSid(owner, expected), "Owner SIDs are not equal\n"); + ok_(__FILE__, line)(!owner_defaulted, "Defaulted is true\n"); + + HeapFree(GetProcessHeap(), 0, queriedSD); +} + +static void test_group_equal(HANDLE Handle, PSID expected, int line) +{ + BOOL res; + SECURITY_DESCRIPTOR *queriedSD = NULL; + PSID group; + BOOL group_defaulted; + + queriedSD = test_get_security_descriptor( Handle, line ); + + res = GetSecurityDescriptorGroup(queriedSD, &group, &group_defaulted); + ok_(__FILE__, line)(res, "GetSecurityDescriptorGroup failed with error %d\n", GetLastError()); + + ok_(__FILE__, line)(EqualSid(group, expected), "Group SIDs are not equal\n"); + ok_(__FILE__, line)(!group_defaulted, "Defaulted is true\n"); + + HeapFree(GetProcessHeap(), 0, queriedSD); +} + static void test_sid(void) { struct sidRef refs[] = { @@ -2504,6 +2540,8 @@ static void test_process_security(void) SECURITY_ATTRIBUTES psa; HANDLE token, event; DWORD size; + SID_IDENTIFIER_AUTHORITY SIDAuthWorld = { SECURITY_WORLD_SID_AUTHORITY }; + PSID EveryoneSid = NULL;
Acl = HeapAlloc(GetProcessHeap(), 0, 256); res = InitializeAcl(Acl, 256, ACL_REVISION); @@ -2515,6 +2553,9 @@ static void test_process_security(void) } ok(res, "InitializeAcl failed with error %d\n", GetLastError());
+ res = AllocateAndInitializeSid( &SIDAuthWorld, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, &EveryoneSid); + ok(res, "AllocateAndInitializeSid failed with error %d\n", GetLastError()); + /* get owner from the token we might be running as a user not admin */ res = OpenProcessToken( GetCurrentProcess(), MAXIMUM_ALLOWED, &token ); ok(res, "OpenProcessToken failed with error %d\n", GetLastError()); @@ -2581,12 +2622,31 @@ static void test_process_security(void) res = SetSecurityDescriptorOwner(SecurityDescriptor, AdminSid, FALSE); ok(res, "SetSecurityDescriptorOwner failed with error %d\n", GetLastError()); CHECK_SET_SECURITY( event, OWNER_SECURITY_INFORMATION, ERROR_SUCCESS ); + test_owner_equal( event, AdminSid, __LINE__ ); + + res = SetSecurityDescriptorGroup(SecurityDescriptor, EveryoneSid, FALSE); + ok(res, "SetSecurityDescriptorGroup failed with error %d\n", GetLastError()); + CHECK_SET_SECURITY( event, GROUP_SECURITY_INFORMATION, ERROR_SUCCESS ); + test_group_equal( event, EveryoneSid, __LINE__ ); + + res = SetSecurityDescriptorDacl(SecurityDescriptor, TRUE, Acl, FALSE); + ok(res, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError()); + CHECK_SET_SECURITY( event, DACL_SECURITY_INFORMATION, ERROR_SUCCESS ); + /* setting a dacl should not change the owner or group */ + test_owner_equal( event, AdminSid, __LINE__ ); + test_group_equal( event, EveryoneSid, __LINE__ ); + + /* Test again with a different SID in case the previous SID also happens to + * be the one that is incorrectly replacing the group. */ res = SetSecurityDescriptorGroup(SecurityDescriptor, UsersSid, FALSE); ok(res, "SetSecurityDescriptorGroup failed with error %d\n", GetLastError()); CHECK_SET_SECURITY( event, GROUP_SECURITY_INFORMATION, ERROR_SUCCESS ); + test_group_equal( event, UsersSid, __LINE__ ); + res = SetSecurityDescriptorDacl(SecurityDescriptor, TRUE, Acl, FALSE); ok(res, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError()); CHECK_SET_SECURITY( event, DACL_SECURITY_INFORMATION, ERROR_SUCCESS ); + test_group_equal( event, UsersSid, __LINE__ );
sprintf(buffer, "%s tests/security.c test", myARGV[0]); memset(&startup, 0, sizeof(startup)); diff --git a/server/object.c b/server/object.c index 021c741..11ef0ce 100644 --- a/server/object.c +++ b/server/object.c @@ -436,18 +436,32 @@ int default_set_sd( struct object *obj, const struct security_descriptor *sd,
new_sd.control = sd->control & ~SE_SELF_RELATIVE;
- owner = sd_get_owner( sd ); - if (set_info & OWNER_SECURITY_INFORMATION && owner) + if (set_info & OWNER_SECURITY_INFORMATION && sd->owner_len) + { + owner = sd_get_owner( sd ); new_sd.owner_len = sd->owner_len; + } + else if (obj->sd && obj->sd->owner_len) + { + owner = sd_get_owner( obj->sd ); + new_sd.owner_len = obj->sd->owner_len; + } else { owner = token_get_user( current->process->token ); new_sd.owner_len = security_sid_len( owner ); }
- group = sd_get_group( sd ); - if (set_info & GROUP_SECURITY_INFORMATION && group) + if (set_info & GROUP_SECURITY_INFORMATION && sd->group_len) + { + group = sd_get_group( sd ); new_sd.group_len = sd->group_len; + } + else if (obj->sd && obj->sd->group_len) + { + group = sd_get_group( obj->sd ); + new_sd.group_len = obj->sd->group_len; + } else { group = token_get_primary_group( current->process->token );
-
Alexandre Julliard