[PATCH v2 0/5] MR2737: ntdll: Fix return value of RtlInitializeSid().
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- v2: ntdll/tests: Add some RtlValidSecurityDescriptor() tests. ntdll: Fix RtlValidSecurityDescriptor() return value. ntdll: Fix subauthority count check in RtlInitializeSid(). ntdll/tests: Add some tests for RtlInitializeSid(). ntdll: Fix return value of RtlInitializeSid(). https://gitlab.winehq.org/wine/wine/-/merge_requests/2737
From: Ake Rehnman <ake.rehnman(a)gmail.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/kernelbase/security.c | 2 +- dlls/ntdll/sec.c | 17 +++-------------- include/winternl.h | 2 +- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/dlls/kernelbase/security.c b/dlls/kernelbase/security.c index 04898f81b06..851cc98587d 100644 --- a/dlls/kernelbase/security.c +++ b/dlls/kernelbase/security.c @@ -411,7 +411,7 @@ BOOL WINAPI GetWindowsAccountDomainSid( PSID sid, PSID domain_sid, DWORD *size ) */ BOOL WINAPI InitializeSid ( PSID sid, PSID_IDENTIFIER_AUTHORITY auth, BYTE count ) { - return RtlInitializeSid( sid, auth, count ); + return set_ntstatus(RtlInitializeSid( sid, auth, count )); } /****************************************************************************** diff --git a/dlls/ntdll/sec.c b/dlls/ntdll/sec.c index 51308384ee8..98afc4edd3d 100644 --- a/dlls/ntdll/sec.c +++ b/dlls/ntdll/sec.c @@ -294,19 +294,8 @@ DWORD WINAPI RtlLengthSid(PSID pSid) /************************************************************************** * RtlInitializeSid [NTDLL.@] - * - * Initialise a SID. - * - * PARAMS - * pSid [I] SID to initialise - * pIdentifierAuthority [I] Identifier Authority - * nSubAuthorityCount [I] Number of Sub Authorities - * - * RETURNS - * Success: TRUE. pSid is initialised with the details given. - * Failure: FALSE, if nSubAuthorityCount is >= SID_MAX_SUB_AUTHORITIES. */ -BOOL WINAPI RtlInitializeSid( +NTSTATUS WINAPI RtlInitializeSid( PSID pSid, PSID_IDENTIFIER_AUTHORITY pIdentifierAuthority, BYTE nSubAuthorityCount) @@ -315,7 +304,7 @@ BOOL WINAPI RtlInitializeSid( SID* pisid=pSid; if (nSubAuthorityCount >= SID_MAX_SUB_AUTHORITIES) - return FALSE; + return STATUS_INVALID_PARAMETER; pisid->Revision = SID_REVISION; pisid->SubAuthorityCount = nSubAuthorityCount; @@ -325,7 +314,7 @@ BOOL WINAPI RtlInitializeSid( for (i = 0; i < nSubAuthorityCount; i++) *RtlSubAuthoritySid(pSid, i) = 0; - return TRUE; + return STATUS_SUCCESS; } /************************************************************************** diff --git a/include/winternl.h b/include/winternl.h index dfdb8f23ec6..43ffcb3b704 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4607,7 +4607,7 @@ NTSYSAPI NTSTATUS WINAPI RtlInitializeExtendedContext2(void*,ULONG,CONTEXT_EX** NTSYSAPI void WINAPI RtlInitializeHandleTable(ULONG,ULONG,RTL_HANDLE_TABLE *); NTSYSAPI void WINAPI RtlInitializeResource(LPRTL_RWLOCK); NTSYSAPI void WINAPI RtlInitializeSRWLock(RTL_SRWLOCK*); -NTSYSAPI BOOL WINAPI RtlInitializeSid(PSID,PSID_IDENTIFIER_AUTHORITY,BYTE); +NTSYSAPI NTSTATUS WINAPI RtlInitializeSid(PSID,PSID_IDENTIFIER_AUTHORITY,BYTE); NTSYSAPI NTSTATUS WINAPI RtlInt64ToUnicodeString(ULONGLONG,ULONG,UNICODE_STRING *); NTSYSAPI NTSTATUS WINAPI RtlIntegerToChar(ULONG,ULONG,ULONG,PCHAR); NTSYSAPI NTSTATUS WINAPI RtlIntegerToUnicodeString(ULONG,ULONG,UNICODE_STRING *); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2737
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ntdll/tests/rtl.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index ba6fea67937..c075cae4ba2 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -3608,6 +3608,24 @@ static void test_RtlFirstFreeAce(void) HeapFree(GetProcessHeap(), 0, acl); } +static void test_RtlInitializeSid(void) +{ + SID_IDENTIFIER_AUTHORITY sid_ident = { SECURITY_NT_AUTHORITY }; + char buffer[SECURITY_MAX_SID_SIZE]; + PSID sid = (PSID)&buffer; + NTSTATUS status; + + status = RtlInitializeSid(sid, &sid_ident, 1); + ok(!status, "Unexpected status %#lx.\n", status); + + status = RtlInitializeSid(sid, &sid_ident, SID_MAX_SUB_AUTHORITIES); + todo_wine + ok(!status, "Unexpected status %#lx.\n", status); + + status = RtlInitializeSid(sid, &sid_ident, SID_MAX_SUB_AUTHORITIES + 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %#lx.\n", status); +} + START_TEST(rtl) { InitFunctionPtrs(); @@ -3652,4 +3670,5 @@ START_TEST(rtl) test_DbgPrint(); test_RtlDestroyHeap(); test_RtlFirstFreeAce(); + test_RtlInitializeSid(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2737
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/ntdll/sec.c | 2 +- dlls/ntdll/tests/rtl.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/ntdll/sec.c b/dlls/ntdll/sec.c index 98afc4edd3d..8cadf7b5d60 100644 --- a/dlls/ntdll/sec.c +++ b/dlls/ntdll/sec.c @@ -303,7 +303,7 @@ NTSTATUS WINAPI RtlInitializeSid( int i; SID* pisid=pSid; - if (nSubAuthorityCount >= SID_MAX_SUB_AUTHORITIES) + if (nSubAuthorityCount > SID_MAX_SUB_AUTHORITIES) return STATUS_INVALID_PARAMETER; pisid->Revision = SID_REVISION; diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index c075cae4ba2..269d26c8a16 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -3619,7 +3619,6 @@ static void test_RtlInitializeSid(void) ok(!status, "Unexpected status %#lx.\n", status); status = RtlInitializeSid(sid, &sid_ident, SID_MAX_SUB_AUTHORITIES); - todo_wine ok(!status, "Unexpected status %#lx.\n", status); status = RtlInitializeSid(sid, &sid_ident, SID_MAX_SUB_AUTHORITIES + 1); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2737
From: Ake Rehnman <ake.rehnman(a)gmail.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/kernelbase/security.c | 5 ++++- dlls/ntdll/sec.c | 20 +++----------------- include/winternl.h | 2 +- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/dlls/kernelbase/security.c b/dlls/kernelbase/security.c index 851cc98587d..bf653933ad6 100644 --- a/dlls/kernelbase/security.c +++ b/dlls/kernelbase/security.c @@ -1105,7 +1105,10 @@ BOOL WINAPI InitializeSecurityDescriptor( PSECURITY_DESCRIPTOR descr, DWORD revi */ BOOL WINAPI IsValidSecurityDescriptor( PSECURITY_DESCRIPTOR descr ) { - return set_ntstatus( RtlValidSecurityDescriptor( descr )); + if (!RtlValidSecurityDescriptor( descr )) + return set_ntstatus(STATUS_INVALID_SECURITY_DESCR); + + return TRUE; } /****************************************************************************** diff --git a/dlls/ntdll/sec.c b/dlls/ntdll/sec.c index 8cadf7b5d60..34e5df7a533 100644 --- a/dlls/ntdll/sec.c +++ b/dlls/ntdll/sec.c @@ -534,25 +534,11 @@ NTSTATUS WINAPI RtlCopySecurityDescriptor(PSECURITY_DESCRIPTOR pSourceSD, PSECUR /************************************************************************** * RtlValidSecurityDescriptor [NTDLL.@] - * - * Determine if a SECURITY_DESCRIPTOR is valid. - * - * PARAMS - * SecurityDescriptor [I] Descriptor to check. - * - * RETURNS - * Success: STATUS_SUCCESS. - * Failure: STATUS_INVALID_SECURITY_DESCR or STATUS_UNKNOWN_REVISION. */ -NTSTATUS WINAPI RtlValidSecurityDescriptor( - PSECURITY_DESCRIPTOR SecurityDescriptor) +BOOLEAN WINAPI RtlValidSecurityDescriptor(PSECURITY_DESCRIPTOR descriptor) { - if ( ! SecurityDescriptor ) - return STATUS_INVALID_SECURITY_DESCR; - if ( ((SECURITY_DESCRIPTOR*)SecurityDescriptor)->Revision != SECURITY_DESCRIPTOR_REVISION ) - return STATUS_UNKNOWN_REVISION; - - return STATUS_SUCCESS; + SECURITY_DESCRIPTOR *sd = descriptor; + return sd && sd->Revision == SECURITY_DESCRIPTOR_REVISION; } /************************************************************************** diff --git a/include/winternl.h b/include/winternl.h index 43ffcb3b704..5ad1d2fa7e2 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4763,7 +4763,7 @@ NTSYSAPI void WINAPI RtlUpperString(STRING *,const STRING *); NTSYSAPI void WINAPI RtlUserThreadStart(PRTL_THREAD_START_ROUTINE,void*); NTSYSAPI BOOLEAN WINAPI RtlValidAcl(PACL); NTSYSAPI BOOLEAN WINAPI RtlValidRelativeSecurityDescriptor(PSECURITY_DESCRIPTOR,ULONG,SECURITY_INFORMATION); -NTSYSAPI NTSTATUS WINAPI RtlValidSecurityDescriptor(PSECURITY_DESCRIPTOR); +NTSYSAPI BOOLEAN WINAPI RtlValidSecurityDescriptor(PSECURITY_DESCRIPTOR); NTSYSAPI BOOLEAN WINAPI RtlValidSid(PSID); NTSYSAPI BOOLEAN WINAPI RtlValidateHeap(HANDLE,ULONG,LPCVOID); NTSYSAPI NTSTATUS WINAPI RtlVerifyVersionInfo(const RTL_OSVERSIONINFOEXW*,DWORD,DWORDLONG); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2737
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/advapi32/tests/security.c | 29 +++++++++++++++++++++++++++++ dlls/ntdll/tests/rtl.c | 24 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 8b4a868ee11..bef0d2d5f2c 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -8560,6 +8560,34 @@ static void test_group_as_file_owner(void) ok(ret, "got error %lu\n", GetLastError()); } +static void test_IsValidSecurityDescriptor(void) +{ + SECURITY_DESCRIPTOR *sd; + BOOL ret; + + SetLastError(0xdeadbeef); + ret = IsValidSecurityDescriptor(NULL); + ok(!ret, "Unexpected return value %d.\n", ret); + ok(GetLastError() == ERROR_INVALID_SECURITY_DESCR, "Unexpected error %ld.\n", GetLastError()); + + sd = calloc(1, SECURITY_DESCRIPTOR_MIN_LENGTH); + + SetLastError(0xdeadbeef); + ret = IsValidSecurityDescriptor(sd); + ok(!ret, "Unexpected return value %d.\n", ret); + ok(GetLastError() == ERROR_INVALID_SECURITY_DESCR, "Unexpected error %ld.\n", GetLastError()); + + ret = InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION); + ok(ret, "Unexpected return value %d, error %ld.\n", ret, GetLastError()); + + SetLastError(0xdeadbeef); + ret = IsValidSecurityDescriptor(sd); + ok(ret, "Unexpected return value %d.\n", ret); + ok(GetLastError() == 0xdeadbeef, "Unexpected error %ld.\n", GetLastError()); + + free(sd); +} + START_TEST(security) { init(); @@ -8629,6 +8657,7 @@ START_TEST(security) test_GetKernelObjectSecurity(); test_elevation(); test_group_as_file_owner(); + test_IsValidSecurityDescriptor(); /* Must be the last test, modifies process token */ test_token_security_descriptor(); diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index 269d26c8a16..b1ef492627a 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -3625,6 +3625,29 @@ static void test_RtlInitializeSid(void) ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %#lx.\n", status); } +static void test_RtlValidSecurityDescriptor(void) +{ + SECURITY_DESCRIPTOR *sd; + NTSTATUS status; + BOOLEAN ret; + + ret = RtlValidSecurityDescriptor(NULL); + ok(!ret, "Unexpected return value %d.\n", ret); + + sd = calloc(1, SECURITY_DESCRIPTOR_MIN_LENGTH); + + ret = RtlValidSecurityDescriptor(sd); + ok(!ret, "Unexpected return value %d.\n", ret); + + status = RtlCreateSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION); + ok(!status, "Unexpected return value %#lx.\n", status); + + ret = RtlValidSecurityDescriptor(sd); + ok(ret, "Unexpected return value %d.\n", ret); + + free(sd); +} + START_TEST(rtl) { InitFunctionPtrs(); @@ -3670,4 +3693,5 @@ START_TEST(rtl) test_RtlDestroyHeap(); test_RtlFirstFreeAce(); test_RtlInitializeSid(); + test_RtlValidSecurityDescriptor(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2737
participants (3)
-
Ake Rehnman -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)