advapi32: Fix GetNamedSecurityInfo with NULL descriptor (try4:skip on page fault)
Alexandre Goujon
ale.goujon at gmail.com
Tue Aug 3 07:03:26 CDT 2010
This should fix bug #18071
Previously, I used #ifdef CRASHES_ON_NT40 (advapi32/tests/crypt.c) but my patch is still pending.
So this time, I used __TRY as seen yesterday with 81557b5e21551150296b811f7617bca4a75a0c70 commit
I think it's, indeed, a better approach.
If this is acceptable, I'll probably send another patch to remove #ifdef CRASHES_ON_NT40 in crypt.c
---
dlls/advapi32/security.c | 69 ++++++++++++++++++++++++++++-----------
dlls/advapi32/tests/security.c | 25 ++++++++++++++
2 files changed, 74 insertions(+), 20 deletions(-)
diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c
index aa7f336..e446250 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..35a5076 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -33,6 +33,7 @@
#include "lmcons.h"
#include "wine/test.h"
+#include "wine/exception.h"
/* copied from Wine winternl.h - not included in the Windows SDK */
typedef enum _OBJECT_INFORMATION_CLASS {
@@ -2628,6 +2629,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 +2662,36 @@ 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);
+
+
+ /* NULL descriptor tests */
+ __TRY
+ error = pGetNamedSecurityInfoA(windows_dir, SE_FILE_OBJECT,DACL_SECURITY_INFORMATION,
+ NULL, NULL, NULL, NULL, NULL);
+ __EXCEPT_PAGE_FAULT
+ win_skip("NT4 does not support a NULL descriptor\n");
+ return;
+ __ENDTRY
+
+ 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 != NULL, "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);
}
static void test_ConvertStringSecurityDescriptor(void)
--
1.7.0.4
More information about the wine-patches
mailing list