From: Alexandre Goujon Subject: advapi32: Fix GetNamedSecurityInfo with NULL descriptor (try4:skip on page fault) Message-Id: <1280837006-30425-1-git-send-email-ale.goujon@gmail.com> Date: Tue, 3 Aug 2010 14:03:26 +0200 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