Nikolay Sivov : ntdll: Check pointers in NtAccessCheck to prevent access violation.
Module: wine Branch: master Commit: 800b05c4b58ad64168c42444918da0e0a6b19f7b URL: http://source.winehq.org/git/wine.git/?a=commit;h=800b05c4b58ad64168c4244491... Author: Nikolay Sivov <bunglehead(a)gmail.com> Date: Tue Jan 13 22:54:32 2009 +0300 ntdll: Check pointers in NtAccessCheck to prevent access violation. --- dlls/advapi32/tests/security.c | 97 ++++++++++++++++++++++++++++++++++++++++ dlls/ntdll/sec.c | 3 + 2 files changed, 100 insertions(+), 0 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 8682cc9..8e8728a 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -107,6 +107,8 @@ static BOOL (WINAPI *pSetSecurityDescriptorControl)(PSECURITY_DESCRIPTOR, SECURI SECURITY_DESCRIPTOR_CONTROL); static DWORD (WINAPI *pGetSecurityInfo)(HANDLE, SE_OBJECT_TYPE, SECURITY_INFORMATION, PSID*, PSID*, PACL*, PACL*, PSECURITY_DESCRIPTOR*); +static NTSTATUS (WINAPI *pNtAccessCheck)(PSECURITY_DESCRIPTOR, HANDLE, ACCESS_MASK, PGENERIC_MAPPING, + PPRIVILEGE_SET, PULONG, PULONG, NTSTATUS*); static HMODULE hmod; static int myARGC; @@ -142,6 +144,7 @@ static void init(void) hntdll = GetModuleHandleA("ntdll.dll"); pNtQueryObject = (void *)GetProcAddress( hntdll, "NtQueryObject" ); + pNtAccessCheck = (void *)GetProcAddress( hntdll, "NtAccessCheck" ); hmod = GetModuleHandle("advapi32.dll"); pAddAccessAllowedAceEx = (void *)GetProcAddress(hmod, "AddAccessAllowedAceEx"); @@ -842,6 +845,7 @@ static void test_AccessCheck(void) HMODULE NtDllModule; BOOLEAN Enabled; DWORD err; + NTSTATUS ntret, ntAccessStatus; NtDllModule = GetModuleHandle("ntdll.dll"); if (!NtDllModule) @@ -917,6 +921,7 @@ static void test_AccessCheck(void) /* Generic access mask */ SetLastError(0xdeadbeef); + Access = AccessStatus = 0xdeadbeef; ret = AccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, PrivSet, &PrivSetLen, &Access, &AccessStatus); err = GetLastError(); @@ -925,7 +930,41 @@ static void test_AccessCheck(void) ok(Access == 0xdeadbeef && AccessStatus == 0xdeadbeef, "Access and/or AccessStatus were changed!\n"); + /* Generic access mask - no privilegeset buffer */ + SetLastError(0xdeadbeef); + Access = AccessStatus = 0xdeadbeef; + ret = AccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + NULL, &PrivSetLen, &Access, &AccessStatus); + err = GetLastError(); + ok(!ret && err == ERROR_NOACCESS, "AccessCheck should have failed " + "with ERROR_NOACCESS, instead of %d\n", err); + ok(Access == 0xdeadbeef && AccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + + /* Generic access mask - no returnlength */ + SetLastError(0xdeadbeef); + Access = AccessStatus = 0xdeadbeef; + ret = AccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + PrivSet, NULL, &Access, &AccessStatus); + err = GetLastError(); + ok(!ret && err == ERROR_NOACCESS, "AccessCheck should have failed " + "with ERROR_NOACCESS, instead of %d\n", err); + ok(Access == 0xdeadbeef && AccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + + /* Generic access mask - no privilegeset buffer, no returnlength */ + SetLastError(0xdeadbeef); + Access = AccessStatus = 0xdeadbeef; + ret = AccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + NULL, NULL, &Access, &AccessStatus); + err = GetLastError(); + ok(!ret && err == ERROR_NOACCESS, "AccessCheck should have failed " + "with ERROR_NOACCESS, instead of %d\n", err); + ok(Access == 0xdeadbeef && AccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + /* sd with no dacl present */ + Access = AccessStatus = 0xdeadbeef; ret = SetSecurityDescriptorDacl(SecurityDescriptor, FALSE, NULL, FALSE); ok(ret, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError()); ret = AccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping, @@ -935,7 +974,63 @@ static void test_AccessCheck(void) "AccessCheck failed to grant access with error %d\n", GetLastError()); + /* sd with no dacl present - no privilegeset buffer */ + SetLastError(0xdeadbeef); + Access = AccessStatus = 0xdeadbeef; + ret = AccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + NULL, &PrivSetLen, &Access, &AccessStatus); + err = GetLastError(); + ok(!ret && err == ERROR_NOACCESS, "AccessCheck should have failed " + "with ERROR_NOACCESS, instead of %d\n", err); + ok(Access == 0xdeadbeef && AccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + + if(pNtAccessCheck) + { + /* Generic access mask - no privilegeset buffer */ + SetLastError(0xdeadbeef); + Access = ntAccessStatus = 0xdeadbeef; + ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + NULL, &PrivSetLen, &Access, &ntAccessStatus); + err = GetLastError(); + ok(ntret == STATUS_ACCESS_VIOLATION, + "NtAccessCheck should have failed with STATUS_ACCESS_VIOLATION, got %x\n", ntret); + ok(err == 0xdeadbeef, + "NtAccessCheck shouldn't set last error, got %d\n", err); + ok(Access == 0xdeadbeef && ntAccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + + /* Generic access mask - no returnlength */ + SetLastError(0xdeadbeef); + Access = ntAccessStatus = 0xdeadbeef; + ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + PrivSet, NULL, &Access, &ntAccessStatus); + err = GetLastError(); + ok(ntret == STATUS_ACCESS_VIOLATION, + "NtAccessCheck should have failed with STATUS_ACCESS_VIOLATION, got %x\n", ntret); + ok(err == 0xdeadbeef, + "NtAccessCheck shouldn't set last error, got %d\n", err); + ok(Access == 0xdeadbeef && ntAccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + + /* Generic access mask - no privilegeset buffer, no returnlength */ + SetLastError(0xdeadbeef); + Access = ntAccessStatus = 0xdeadbeef; + ntret = pNtAccessCheck(SecurityDescriptor, Token, GENERIC_READ, &Mapping, + NULL, NULL, &Access, &ntAccessStatus); + err = GetLastError(); + ok(ntret == STATUS_ACCESS_VIOLATION, + "NtAccessCheck should have failed with STATUS_ACCESS_VIOLATION, got %x\n", ntret); + ok(err == 0xdeadbeef, + "NtAccessCheck shouldn't set last error, got %d\n", err); + ok(Access == 0xdeadbeef && ntAccessStatus == 0xdeadbeef, + "Access and/or AccessStatus were changed!\n"); + } + else + win_skip("NtAccessCheck unavailable. Skipping.\n"); + /* sd with NULL dacl */ + Access = AccessStatus = 0xdeadbeef; ret = SetSecurityDescriptorDacl(SecurityDescriptor, TRUE, NULL, FALSE); ok(ret, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError()); ret = AccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping, @@ -963,6 +1058,7 @@ static void test_AccessCheck(void) ok(res, "AddAccessDeniedAce failed with error %d\n", GetLastError()); /* sd with dacl */ + Access = AccessStatus = 0xdeadbeef; ret = AccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping, PrivSet, &PrivSetLen, &Access, &AccessStatus); ok(ret, "AccessCheck failed with error %d\n", GetLastError()); @@ -980,6 +1076,7 @@ static void test_AccessCheck(void) /* Access denied by SD */ SetLastError(0xdeadbeef); + Access = AccessStatus = 0xdeadbeef; ret = AccessCheck(SecurityDescriptor, Token, KEY_WRITE, &Mapping, PrivSet, &PrivSetLen, &Access, &AccessStatus); ok(ret, "AccessCheck failed with error %d\n", GetLastError()); diff --git a/dlls/ntdll/sec.c b/dlls/ntdll/sec.c index a764e21..6a8a0ce 100644 --- a/dlls/ntdll/sec.c +++ b/dlls/ntdll/sec.c @@ -1562,6 +1562,9 @@ NtAccessCheck( SecurityDescriptor, ClientToken, DesiredAccess, GenericMapping, PrivilegeSet, ReturnLength, GrantedAccess, AccessStatus); + if (!PrivilegeSet || !ReturnLength) + return STATUS_ACCESS_VIOLATION; + SERVER_START_REQ( access_check ) { struct security_descriptor sd;
participants (1)
-
Alexandre Julliard