Zebediah Figura : ntdll: Check the output buffer length in NtAccessCheck().
Alexandre Julliard
julliard at winehq.org
Mon Feb 8 15:46:41 CST 2021
Module: wine
Branch: master
Commit: be98f67f10c26b46acbf2fe98ea764c27efd3612
URL: https://source.winehq.org/git/wine.git/?a=commit;h=be98f67f10c26b46acbf2fe98ea764c27efd3612
Author: Zebediah Figura <z.figura12 at gmail.com>
Date: Sun Feb 7 19:50:11 2021 -0600
ntdll: Check the output buffer length in NtAccessCheck().
Based on a patch by Qian Hong.
Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>
---
dlls/advapi32/tests/security.c | 20 ++++++--------------
dlls/ntdll/unix/security.c | 14 ++++++++++----
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index 764e0bb5ef6..e6d2f565943 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -1368,11 +1368,11 @@ static void test_AccessCheck(void)
ntret = pNtAccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping,
PrivSet, &ntPrivSetLen, &Access, &ntAccessStatus);
err = GetLastError();
- todo_wine ok(ntret == STATUS_BUFFER_TOO_SMALL,
+ ok(ntret == STATUS_BUFFER_TOO_SMALL,
"NtAccessCheck should have failed with STATUS_BUFFER_TOO_SMALL, got %x\n", ntret);
ok(err == 0xdeadbeef,
"NtAccessCheck shouldn't set last error, got %d\n", err);
- todo_wine ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
+ ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
"Access and/or AccessStatus were changed!\n");
ok(ntPrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", ntPrivSetLen);
@@ -1383,11 +1383,11 @@ static void test_AccessCheck(void)
ntret = pNtAccessCheck(SecurityDescriptor, Token, KEY_READ, &Mapping,
PrivSet, &ntPrivSetLen, &Access, &ntAccessStatus);
err = GetLastError();
- todo_wine ok(ntret == STATUS_BUFFER_TOO_SMALL,
+ ok(ntret == STATUS_BUFFER_TOO_SMALL,
"NtAccessCheck should have failed with STATUS_BUFFER_TOO_SMALL, got %x\n", ntret);
ok(err == 0xdeadbeef,
"NtAccessCheck shouldn't set last error, got %d\n", err);
- todo_wine ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
+ ok(Access == 0x1abe11ed && ntAccessStatus == 0x1abe11ed,
"Access and/or AccessStatus were changed!\n");
ok(ntPrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", ntPrivSetLen);
}
@@ -1517,11 +1517,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);
ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen);
-todo_wine
ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed,
"Access and/or AccessStatus were changed!\n");
@@ -1532,11 +1530,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);
ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen);
-todo_wine
ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed,
"Access and/or AccessStatus were changed!\n");
@@ -1544,16 +1540,16 @@ todo_wine
SetLastError(0xdeadbeef);
Access = AccessStatus = 0x1abe11ed;
PrivSetLen = sizeof(PRIVILEGE_SET) - 1;
+ PrivSet->PrivilegeCount = 0xdeadbeef;
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);
ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen);
-todo_wine
ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed,
"Access and/or AccessStatus were changed!\n");
+ ok(PrivSet->PrivilegeCount == 0xdeadbeef, "buffer contents should not be changed\n");
/* Valid PrivSet with minimal sufficient PrivSetLen */
SetLastError(0xdeadbeef);
@@ -1630,11 +1626,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);
ok(PrivSetLen == sizeof(PRIVILEGE_SET), "PrivSetLen returns %d\n", PrivSetLen);
- todo_wine
ok(Access == 0x1abe11ed && AccessStatus == 0x1abe11ed,
"Access and/or AccessStatus were changed!\n");
@@ -1645,11 +1639,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);
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/unix/security.c b/dlls/ntdll/unix/security.c
index 0134b80e148..8838ca13ee4 100644
--- a/dlls/ntdll/unix/security.c
+++ b/dlls/ntdll/unix/security.c
@@ -709,11 +709,13 @@ NTSTATUS WINAPI NtAccessCheck( PSECURITY_DESCRIPTOR descr, HANDLE token, ACCESS_
data_size_t len;
OBJECT_ATTRIBUTES attr;
NTSTATUS status;
+ ULONG priv_len;
TRACE( "(%p, %p, %08x, %p, %p, %p, %p, %p)\n",
descr, token, access, mapping, privs, retlen, access_granted, access_status );
if (!privs || !retlen) return STATUS_ACCESS_VIOLATION;
+ priv_len = *retlen;
/* reuse the object attribute SD marshalling */
InitializeObjectAttributes( &attr, NULL, 0, 0, descr );
@@ -728,16 +730,20 @@ NTSTATUS WINAPI NtAccessCheck( PSECURITY_DESCRIPTOR descr, HANDLE token, ACCESS_
req->mapping.exec = mapping->GenericExecute;
req->mapping.all = mapping->GenericAll;
wine_server_add_data( req, objattr + 1, objattr->sd_len );
- wine_server_set_reply( req, privs->Privilege, *retlen - offsetof( PRIVILEGE_SET, Privilege ) );
+ wine_server_set_reply( req, privs->Privilege, priv_len - offsetof( PRIVILEGE_SET, Privilege ) );
status = wine_server_call( req );
if (status == STATUS_SUCCESS)
{
*retlen = max( offsetof( PRIVILEGE_SET, Privilege ) + reply->privileges_len, sizeof(PRIVILEGE_SET) );
- privs->PrivilegeCount = reply->privileges_len / sizeof(LUID_AND_ATTRIBUTES);
- *access_status = reply->access_status;
- *access_granted = reply->access_granted;
+ if (priv_len >= *retlen)
+ {
+ privs->PrivilegeCount = reply->privileges_len / sizeof(LUID_AND_ATTRIBUTES);
+ *access_status = reply->access_status;
+ *access_granted = reply->access_granted;
+ }
+ else status = STATUS_BUFFER_TOO_SMALL;
}
}
SERVER_END_REQ;
More information about the wine-cvs
mailing list