On 4/24/19 8:21 PM, Vijay Kiran Kamuju wrote:
On Tue, Apr 23, 2019 at 4:10 PM Vijay Kiran Kamuju infyquest@gmail.com wrote:
On Tue, Apr 23, 2019 at 4:01 PM Zhiyi Zhang zzhang@codeweavers.com wrote:
On 4/23/19 9:31 PM, Vijay Kiran Kamuju wrote:
From: Qian Hong qhong@codeweavers.com
Signed-off-by: Qian Hong qhong@codeweavers.com Signed-off-by: Vijay Kiran Kamuju infyquest@gmail.com
dlls/advapi32/tests/security.c | 8 -------- dlls/ntdll/sec.c | 6 ++++++ 2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index d9cae64da8b..d886ab713f3 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -1454,10 +1454,8 @@ static void test_AccessCheck(void) ret = AccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping, 0, &PrivSetLen, &Access, &AccessStatus); err = GetLastError(); -todo_wine ok(!ret && err == ERROR_INSUFFICIENT_BUFFER, "AccessCheck should have " "failed with ERROR_INSUFFICIENT_BUFFER, instead of %d\n", err); -todo_wine ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen); ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed, "Access and/or AccessStatus were changed!\n"); @@ -1508,12 +1506,9 @@ todo_wine ret = AccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping, PrivSet, &PrivSetLen, &Access, &AccessStatus); err = GetLastError(); -todo_wine ok(!ret && err == ERROR_INSUFFICIENT_BUFFER, "AccessCheck should have " "failed with ERROR_INSUFFICIENT_BUFFER, instead of %d\n", err); -todo_wine ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen); -todo_wine ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed, "Access and/or AccessStatus were changed!\n");
@@ -1625,12 +1620,9 @@ todo_wine ret = AccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping, PrivSet, &PrivSetLen, &Access, &AccessStatus); err = GetLastError();
- todo_wine ok(!ret && err == ERROR_INSUFFICIENT_BUFFER, "AccessCheck should have " "failed with ERROR_INSUFFICIENT_BUFFER, instead of %d\n", err);
- todo_wine ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen);
- todo_wine ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed, "Access and/or AccessStatus were changed!\n");
diff --git a/dlls/ntdll/sec.c b/dlls/ntdll/sec.c index 02fc77dc1cc..f7f1a925770 100644 --- a/dlls/ntdll/sec.c +++ b/dlls/ntdll/sec.c @@ -1670,6 +1670,12 @@ NtAccessCheck( if (!PrivilegeSet || !ReturnLength) return STATUS_ACCESS_VIOLATION;
- if (*ReturnLength == 0)
I think testing *ReturnLength < sizeof(PRIVILEGE_SET) is more appropriate. You should add some tests to verify its behavior, e.g., When *ReturnLength = sizeof(PRIVILEGE_SET)-1
I will check this behavior tomorrow with tests, I was trying to understand the tests today. Sent a wrong test patch earlier :)
I have checked, there are already tests for that. If we add the check you requested, it fails the tests for advapi32AccessCheck functions. It seems the advapi32.AccessCheck and ntdll.NtAccessCheck work differently.
As you said it seems advapi32.AccessCheck and ntdll.NtAccessCheck work differently. The tests you mention that checked size are for advapi32.AccessCheck, not for NtAccessCheck. You should at least test sizeof(PRIVILEGE_SET)-1 for NtAccessCheck, just in case.
- {
*ReturnLength = sizeof(PRIVILEGE_SET);return STATUS_BUFFER_TOO_SMALL;- }
- SERVER_START_REQ( access_check ) { struct security_descriptor sd;