[PATCH 3/3] ntdll: Check the output buffer length in NtAccessCheck().
Zebediah Figura
z.figura12 at gmail.com
Sun Feb 7 19:50:11 CST 2021
Based on a patch by Qian Hong.
Signed-off-by: Zebediah Figura <z.figura12 at gmail.com>
---
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 f6c37bdd738..991f57f1fc7 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -1361,11 +1361,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);
@@ -1376,11 +1376,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);
}
@@ -1510,11 +1510,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");
@@ -1525,11 +1523,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");
@@ -1537,16 +1533,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);
@@ -1623,11 +1619,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");
@@ -1638,11 +1632,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;
--
2.20.1
More information about the wine-devel
mailing list