advapi32: Fix GetNamedSecurityInfo with NULL descriptor [resend]

Alexandre Goujon ale.goujon at gmail.com
Sun Jul 18 05:16:17 CDT 2010


---
 dlls/advapi32/security.c       |   69 ++++++++++++++++++++++++++++-----------
 dlls/advapi32/tests/security.c |   27 +++++++++++++++
 2 files changed, 76 insertions(+), 20 deletions(-)

diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c
index 821fb86..1c210d6 100644
--- a/dlls/advapi32/security.c
+++ b/dlls/advapi32/security.c
@@ -5272,15 +5272,24 @@ DWORD WINAPI GetNamedSecurityInfoW( LPWSTR name, SE_OBJECT_TYPE type,
     PACL* sacl, PSECURITY_DESCRIPTOR* descriptor )
 {
     DWORD needed, offset;
-    SECURITY_DESCRIPTOR_RELATIVE *relative;
+    SECURITY_DESCRIPTOR_RELATIVE *relative = NULL;
     BYTE *buffer;
 
     TRACE( "%s %d %d %p %p %p %p %p\n", debugstr_w(name), type, info, owner,
            group, dacl, sacl, descriptor );
 
-    if (!name || !descriptor) return ERROR_INVALID_PARAMETER;
+    /* A NULL descriptor is allowed if any one of the other pointers is not NULL */
+    if (!name || !(owner||group||dacl||sacl||descriptor) ) return ERROR_INVALID_PARAMETER;
 
-    needed = sizeof(SECURITY_DESCRIPTOR_RELATIVE);
+    /* If no descriptor, we have to check that there's a pointer for the requested information */
+    if( !descriptor && (
+        ((info & OWNER_SECURITY_INFORMATION) && !owner)
+    ||  ((info & GROUP_SECURITY_INFORMATION) && !group)
+    ||  ((info & DACL_SECURITY_INFORMATION)  && !dacl)
+    ||  ((info & SACL_SECURITY_INFORMATION)  && !sacl)  ))
+        return ERROR_INVALID_PARAMETER;
+
+    needed = !descriptor ? 0 : sizeof(SECURITY_DESCRIPTOR_RELATIVE);
     if (info & OWNER_SECURITY_INFORMATION)
         needed += sizeof(sidWorld);
     if (info & GROUP_SECURITY_INFORMATION)
@@ -5290,25 +5299,37 @@ DWORD WINAPI GetNamedSecurityInfoW( LPWSTR name, SE_OBJECT_TYPE type,
     if (info & SACL_SECURITY_INFORMATION)
         needed += WINE_SIZE_OF_WORLD_ACCESS_ACL;
 
-    /* must be freed by caller */
-    *descriptor = HeapAlloc( GetProcessHeap(), 0, needed );
-    if (!*descriptor) return ERROR_NOT_ENOUGH_MEMORY;
+    if(descriptor)
+    {
+        /* must be freed by caller */
+        *descriptor = HeapAlloc( GetProcessHeap(), 0, needed );
+        if (!*descriptor) return ERROR_NOT_ENOUGH_MEMORY;
 
-    if (!InitializeSecurityDescriptor( *descriptor, SECURITY_DESCRIPTOR_REVISION ))
+        if (!InitializeSecurityDescriptor( *descriptor, SECURITY_DESCRIPTOR_REVISION ))
+        {
+            HeapFree( GetProcessHeap(), 0, *descriptor );
+            return ERROR_INVALID_SECURITY_DESCR;
+        }
+
+        relative = *descriptor;
+        relative->Control |= SE_SELF_RELATIVE;
+
+        buffer = (BYTE *)relative;
+        offset = sizeof(SECURITY_DESCRIPTOR_RELATIVE);
+    }
+    else
     {
-        HeapFree( GetProcessHeap(), 0, *descriptor );
-        return ERROR_INVALID_SECURITY_DESCR;
+        buffer = (BYTE *)HeapAlloc( GetProcessHeap(), 0, needed );
+        if (!buffer) return ERROR_NOT_ENOUGH_MEMORY;
+        offset = 0;
     }
 
-    relative = *descriptor;
-    relative->Control |= SE_SELF_RELATIVE;
-    buffer = (BYTE *)relative;
-    offset = sizeof(SECURITY_DESCRIPTOR_RELATIVE);
-
     if (info & OWNER_SECURITY_INFORMATION)
     {
         memcpy( buffer + offset, &sidWorld, sizeof(sidWorld) );
-        relative->Owner = offset;
+        /* If we have a descriptor */
+        if(relative)
+            relative->Owner = offset;
         if (owner)
             *owner = buffer + offset;
         offset += sizeof(sidWorld);
@@ -5316,28 +5337,36 @@ DWORD WINAPI GetNamedSecurityInfoW( LPWSTR name, SE_OBJECT_TYPE type,
     if (info & GROUP_SECURITY_INFORMATION)
     {
         memcpy( buffer + offset, &sidWorld, sizeof(sidWorld) );
-        relative->Group = offset;
+        if(relative)
+            relative->Group = offset;
         if (group)
             *group = buffer + offset;
         offset += sizeof(sidWorld);
     }
     if (info & DACL_SECURITY_INFORMATION)
     {
-        relative->Control |= SE_DACL_PRESENT;
         GetWorldAccessACL( (PACL)(buffer + offset) );
-        relative->Dacl = offset;
+        if(relative)
+        {
+            relative->Control |= SE_DACL_PRESENT;
+            relative->Dacl = offset;
+        }
         if (dacl)
             *dacl = (PACL)(buffer + offset);
         offset += WINE_SIZE_OF_WORLD_ACCESS_ACL;
     }
     if (info & SACL_SECURITY_INFORMATION)
     {
-        relative->Control |= SE_SACL_PRESENT;
         GetWorldAccessACL( (PACL)(buffer + offset) );
-        relative->Sacl = offset;
+        if(relative)
+        {
+            relative->Control |= SE_SACL_PRESENT;
+            relative->Sacl = offset;
+        }
         if (sacl)
             *sacl = (PACL)(buffer + offset);
     }
+
     return ERROR_SUCCESS;
 }
 
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index cd4624a..90b1636 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -2628,6 +2628,7 @@ static void test_GetNamedSecurityInfoA(void)
     SECURITY_DESCRIPTOR_CONTROL control;
     PSID owner;
     PSID group;
+    PACL dacl;
     BOOL owner_defaulted;
     BOOL group_defaulted;
     DWORD error;
@@ -2660,13 +2661,39 @@ static void test_GetNamedSecurityInfoA(void)
         broken((control & (SE_SELF_RELATIVE|SE_DACL_PRESENT)) == SE_DACL_PRESENT), /* NT4 */
         "control (0x%x) doesn't have (SE_SELF_RELATIVE|SE_DACL_PRESENT) flags set\n", control);
     ok(revision == SECURITY_DESCRIPTOR_REVISION1, "revision was %d instead of 1\n", revision);
+
     ret = GetSecurityDescriptorOwner(pSecDesc, &owner, &owner_defaulted);
     ok(ret, "GetSecurityDescriptorOwner failed with error %d\n", GetLastError());
     ok(owner != NULL, "owner should not be NULL\n");
+
     ret = GetSecurityDescriptorGroup(pSecDesc, &group, &group_defaulted);
     ok(ret, "GetSecurityDescriptorGroup failed with error %d\n", GetLastError());
     ok(group != NULL, "group should not be NULL\n");
+
     LocalFree(pSecDesc);
+
+
+#ifdef CRASHES_ON_NT40
+    /* NULL descriptor tests */
+
+    error = pGetNamedSecurityInfoA(windows_dir, SE_FILE_OBJECT,DACL_SECURITY_INFORMATION,
+        NULL, NULL, NULL, NULL, NULL);
+    if( error==STATUS_ACCESS_VIOLATION )
+    {
+        win_skip("NT4 crashes with a NULL descriptor");
+        return;
+    }
+    ok(error==ERROR_INVALID_PARAMETER, "GetNamedSecurityInfo failed with error %d\n", error);
+
+    error = pGetNamedSecurityInfoA(windows_dir, SE_FILE_OBJECT,DACL_SECURITY_INFORMATION,
+        NULL, NULL, &dacl, NULL, NULL);
+    ok(!error, "GetNamedSecurityInfo failed with error %d\n", error);
+    ok(dacl, "dacl should not be NULL\n");
+
+    error = pGetNamedSecurityInfoA(windows_dir, SE_FILE_OBJECT,OWNER_SECURITY_INFORMATION,
+        NULL, NULL, &dacl, NULL, NULL);
+    ok(error==ERROR_INVALID_PARAMETER, "GetNamedSecurityInfo failed with error %d\n", error);
+#endif
 }
 
 static void test_ConvertStringSecurityDescriptor(void)
-- 
1.7.0.4




More information about the wine-patches mailing list