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.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 0a4c7860f82074f7c81d6378075e63848b00b98e
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=0a4c7860f82074f7c81d6378075e63848b00b98e

Author: Joris van der Wel <joris at 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 );