Intravision/reactos
1
0
Fork 0
mirror of https://github.com/reactos/reactos.git synced 2025-02-28 19:32:59 +00:00
Code Issues Projects Releases Packages Wiki Activity

[0.4.15][NTOS:SE][NDK][KMTESTS:SE] Fix 'kmtest_.exe SeQueryInfoToken' (#5308)

Browse source 
This backport fixes 'kmtest_.exe SeQueryInfoToken'
on all testers: VBox x86, KVM x86, WHS x86, Win2003_x64.
And according to Thomas description may also prevent a buffer overrun when executing that formerly broken test.
Afterwards all 76 tests of this suite do complete on all those builders.
Before the patch only 74 of those tests succeeded, 2 failed.

The fix is a squashed backport of the following 6 commits from Thomas Faber:
0.4.16-dev-11-g 44bdafa17e [KMTESTS:SE] Fix failing tests (#5308)
0.4.16-dev-10-g bf6af0f52e [NTOS:SE] Mark output parameters as such (#5308)
0.4.16-dev-9-g 156053cafd [NDK] Match AUX_ACCESS_DATA definition with publicly available version. - if you allocated only sizeof(AUX_ACCESS_DATA), the test would crash with a 4 byte buffer overflow. (#5308)
0.4.16-dev-8-g ff410211e9 [KMTESTS:SE] Don't modify internal data structure, this might cause buffer overrun (#5308)
0.4.16-dev-7-g 206df96bc4 [KMTESTS:SE] Correctly allocate PrivilegeSet buffers (#5308)
0.4.16-dev-6-g 64a6bd4c3e [KMTESTS:SE] Avoid use of uninitialized pool and hardcoded offsets (#5308)

WHS x86 before-and-after-state, the after-test had a few fixes from Timos unrelated PR7343 inside unfortunately:
https://reactos.org/testman/compare.php?ids=97640,97871
(This is added to prove the test being wrong)

I tested it also successfully on my local 2k3sp2 x86 with the releases/0.4.15 afterstate, built with RosBEWin2.2.2 GCC8.4.0dbg x86.

Win2003_x64 0.4.16-dev-11-g44bdafa at 2024-09-12 15:19 (after-state):
https://reactos.org/testman/compare.php?ids=97791

0.4.16-dev-5-g2913ef5 vs. 0.4.16-dev-11-g44bdafa vs. 0.4.16-dev-23-g53b304e:
VBox x86 https://reactos.org/testman/compare.php?ids=97795,97806,97877

0.4.16-dev-5-g2913ef5 vs. 0.4.16-dev-20-g144a8b5 vs. 0.4.16-dev-21-g2af6fd4:
KVM x86 https://reactos.org/testman/compare.php?ids=97793,97855,97856

Since we do touch the NTOS and NDK here the fix is not guaranteed to be side-effect-free,
but since we are so early in the RC-phase, I dared to pick it, especially since the alternative would have
been to disable the test altogether in the releases/0.4.15 which would have been a pity, if we can also have it all green everywhere.
This commit is contained in:
Joachim Henze 2023-05-27 11:19:17 -04:00
parent 5f1fcb59e5
commit 3780e42ff9
5 changed files with 96 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

95
modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c
View file

@ -13,6 +13,19 @@
#define NDEBUG
#include <debug.h>
// Copied from PspProcessMapping -- although the values don't matter much for
// the most part.
static GENERIC_MAPPING ProcessGenericMapping =
{
STANDARD_RIGHTS_READ | PROCESS_QUERY_INFORMATION | PROCESS_VM_READ,
STANDARD_RIGHTS_WRITE | PROCESS_CREATE_PROCESS | PROCESS_CREATE_THREAD |
PROCESS_VM_OPERATION | PROCESS_VM_WRITE | PROCESS_DUP_HANDLE |
PROCESS_TERMINATE | PROCESS_SET_QUOTA | PROCESS_SET_INFORMATION |
PROCESS_SUSPEND_RESUME,
STANDARD_RIGHTS_EXECUTE | SYNCHRONIZE,
PROCESS_ALL_ACCESS
};
//------------------------------------------------------------------------------//
// Testing Functions //
//------------------------------------------------------------------------------//
@ -216,14 +229,13 @@ START_TEST(SeQueryInfoToken)
NTSTATUS Status = STATUS_SUCCESS;
PAUX_ACCESS_DATA AuxData = NULL;
PPRIVILEGE_SET NewPrivilegeSet;
ULONG InitialPrivilegeCount;
BOOLEAN Checker;
PPRIVILEGE_SET Privileges = NULL;
PSECURITY_SUBJECT_CONTEXT SubjectContext = NULL;
PACCESS_TOKEN Token = NULL;
PTOKEN_PRIVILEGES TPrivileges;
PVOID Buffer;
POBJECT_TYPE PsProcessType = NULL;
PGENERIC_MAPPING GenericMapping;
ULONG i;
SubjectContext = ExAllocatePool(PagedPool, sizeof(SECURITY_SUBJECT_CONTEXT));
@ -240,14 +252,14 @@ START_TEST(SeQueryInfoToken)
//----------------------------------------------------------------//
AccessState = ExAllocatePool(PagedPool, sizeof(ACCESS_STATE));
PsProcessType = ExAllocatePool(PagedPool, sizeof(OBJECT_TYPE));
AuxData = ExAllocatePool(PagedPool, 0xC8);
GenericMapping = ExAllocatePool(PagedPool, sizeof(GENERIC_MAPPING));
// AUX_ACCESS_DATA gets larger in newer Windows version.
// This is the largest known size, found in Windows 10/11.
AuxData = ExAllocatePoolZero(PagedPool, 0xE0, 'QSmK');
Status = SeCreateAccessState(AccessState,
(PVOID)AuxData,
AuxData,
DesiredAccess,
GenericMapping
&ProcessGenericMapping
);
ok((Status == STATUS_SUCCESS), "SeCreateAccessState failed with Status 0x%08X\n", Status);
@ -267,43 +279,48 @@ START_TEST(SeQueryInfoToken)
// Testing SeAppendPrivileges //
//----------------------------------------------------------------//
AuxData->PrivilegeSet->PrivilegeCount = 1;
InitialPrivilegeCount = AuxData->PrivilegesUsed->PrivilegeCount;
trace("Initial privilege count = %lu\n", InitialPrivilegeCount);
// Testing SeAppendPrivileges. Must change PrivilegeCount to 2 (1 + 1)
NewPrivilegeSet = ExAllocatePool(PagedPool, sizeof(PRIVILEGE_SET));
NewPrivilegeSet = ExAllocatePoolZero(PagedPool,
FIELD_OFFSET(PRIVILEGE_SET, Privilege[1]),
'QSmK');
NewPrivilegeSet->PrivilegeCount = 1;
Status = SeAppendPrivileges(AccessState, NewPrivilegeSet);
ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n");
ok((AuxData->PrivilegeSet->PrivilegeCount == 2),"PrivelegeCount must be 2, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount);
ExFreePool(NewPrivilegeSet);
ok_eq_ulong(AuxData->PrivilegesUsed->PrivilegeCount, InitialPrivilegeCount + 1);
ExFreePoolWithTag(NewPrivilegeSet, 'QSmK');
//----------------------------------------------------------------//
// Testing SeAppendPrivileges. Must change PrivilegeCount to 6 (2 + 4)
NewPrivilegeSet = ExAllocatePool(PagedPool, 4*sizeof(PRIVILEGE_SET));
NewPrivilegeSet = ExAllocatePoolZero(PagedPool,
FIELD_OFFSET(PRIVILEGE_SET, Privilege[4]),
'QSmK');
NewPrivilegeSet->PrivilegeCount = 4;
Status = SeAppendPrivileges(AccessState, NewPrivilegeSet);
ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n");
ok((AuxData->PrivilegeSet->PrivilegeCount == 6),"PrivelegeCount must be 6, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount);
ExFreePool(NewPrivilegeSet);
ok_eq_ulong(AuxData->PrivilegesUsed->PrivilegeCount, InitialPrivilegeCount + 5);
ExFreePoolWithTag(NewPrivilegeSet, 'QSmK');
//----------------------------------------------------------------//
// Testing SePrivilegeCheck //
//----------------------------------------------------------------//
// KPROCESSOR_MODE is set to KernelMode ===> Always return TRUE
ok(SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n");
ok(SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n");
// and call it again
ok(SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n");
ok(SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n");
//----------------------------------------------------------------//
// KPROCESSOR_MODE is set to UserMode. Expect false
ok(!SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck unexpected success with UserMode arg\n");
ok(!SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck unexpected success with UserMode arg\n");
//----------------------------------------------------------------//
@ -311,6 +328,10 @@ START_TEST(SeQueryInfoToken)
// Testing SeFreePrivileges //
//----------------------------------------------------------------//
// FIXME: KernelMode will automatically get all access granted without
// getting Privileges filled in. For UserMode, Privileges will only get
// filled if either WRITE_OWNER or ACCESS_SYSTEM_SECURITY is requested
// and granted. So this doesn't really test SeFreePrivileges.
Privileges = NULL;
Checker = SeAccessCheck(
AccessState->SecurityDescriptor,
@ -319,17 +340,17 @@ START_TEST(SeQueryInfoToken)
AccessState->OriginalDesiredAccess,
AccessState->PreviouslyGrantedAccess,
&Privileges,
(PGENERIC_MAPPING)((PCHAR*)PsProcessType + 52),
&ProcessGenericMapping,
KernelMode,
&AccessMask,
&Status
);
ok(Checker, "Checker is NULL\n");
ok((Privileges != NULL), "Privileges is NULL\n");
ok(Privileges == NULL, "Privileges is not NULL\n");
if (Privileges)
{
trace("AuxData->PrivilegeSet->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n",
AuxData->PrivilegeSet->PrivilegeCount, Privileges->PrivilegeCount);
trace("AuxData->PrivilegesUsed->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n",
AuxData->PrivilegesUsed->PrivilegeCount, Privileges->PrivilegeCount);
}
if (Privileges) SeFreePrivileges(Privileges);
@ -352,25 +373,29 @@ START_TEST(SeQueryInfoToken)
TPrivileges = (PTOKEN_PRIVILEGES)(Buffer);
//trace("TPCount = %u\n\n", TPrivileges->PrivilegeCount);
NewPrivilegeSet = ExAllocatePool(PagedPool, 14*sizeof(PRIVILEGE_SET));
NewPrivilegeSet = ExAllocatePoolZero(PagedPool,
FIELD_OFFSET(PRIVILEGE_SET, Privilege[14]),
'QSmK');
NewPrivilegeSet->PrivilegeCount = 14;
ok((SeAppendPrivileges(AccessState, NewPrivilegeSet)) == STATUS_SUCCESS, "SeAppendPrivileges failed\n");
ok((AuxData->PrivilegeSet->PrivilegeCount == 20),"PrivelegeCount must be 20, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount);
ExFreePool(NewPrivilegeSet);
for (i = 0; i < AuxData->PrivilegeSet->PrivilegeCount; i++)
ok_eq_ulong(AuxData->PrivilegesUsed->PrivilegeCount, InitialPrivilegeCount + 19);
ExFreePoolWithTag(NewPrivilegeSet, 'QSmK');
for (i = 0; i < AuxData->PrivilegesUsed->PrivilegeCount; i++)
{
AuxData->PrivilegeSet->Privilege[i].Attributes = TPrivileges->Privileges[i].Attributes;
AuxData->PrivilegeSet->Privilege[i].Luid = TPrivileges->Privileges[i].Luid;
AuxData->PrivilegesUsed->Privilege[i].Attributes = TPrivileges->Privileges[i].Attributes;
AuxData->PrivilegesUsed->Privilege[i].Luid = TPrivileges->Privileges[i].Luid;
}
//trace("AccessState->privCount = %u\n\n", ((PAUX_ACCESS_DATA)(AccessState->AuxData))->PrivilegeSet->PrivilegeCount);
//trace("AccessState->privCount = %u\n\n", ((PAUX_ACCESS_DATA)(AccessState->AuxData))->PrivilegesUsed->PrivilegeCount);
ok(SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck fails in UserMode, but I wish it will success\n");
ok(SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck fails in UserMode, but I wish it will success\n");
}
}
// Call SeFreePrivileges again
// FIXME: See other SeAccessCheck call above, we're not really testing
// SeFreePrivileges here.
Privileges = NULL;
Checker = SeAccessCheck(
AccessState->SecurityDescriptor,
@ -379,17 +404,17 @@ START_TEST(SeQueryInfoToken)
AccessState->OriginalDesiredAccess,
AccessState->PreviouslyGrantedAccess,
&Privileges,
(PGENERIC_MAPPING)((PCHAR*)PsProcessType + 52),
&ProcessGenericMapping,
KernelMode,
&AccessMask,
&Status
);
ok(Checker, "Checker is NULL\n");
ok((Privileges != NULL), "Privileges is NULL\n");
ok(Privileges == NULL, "Privileges is not NULL\n");
if (Privileges)
{
trace("AuxData->PrivilegeSet->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n",
AuxData->PrivilegeSet->PrivilegeCount, Privileges->PrivilegeCount);
trace("AuxData->PrivilegesUsed->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n",
AuxData->PrivilegesUsed->PrivilegeCount, Privileges->PrivilegeCount);
}
if (Privileges) SeFreePrivileges(Privileges);
@ -402,9 +427,7 @@ START_TEST(SeQueryInfoToken)
SeDeleteAccessState(AccessState);
if (GenericMapping) ExFreePool(GenericMapping);
if (PsProcessType) ExFreePool(PsProcessType);
if (SubjectContext) ExFreePool(SubjectContext);
if (AuxData) ExFreePool(AuxData);
if (AuxData) ExFreePoolWithTag(AuxData, 'QSmK');
if (AccessState) ExFreePool(AccessState);
}

6
ntoskrnl/ob/obhandle.c
View file

@ -1647,8 +1647,8 @@ ObpCreateHandle(IN OB_OPEN_REASON OpenReason,
if (OpenReason == ObCreateHandle)
{
/* Check if we need to audit the privileges */
if ((AuxData->PrivilegeSet) &&
(AuxData->PrivilegeSet->PrivilegeCount))
if ((AuxData->PrivilegesUsed) &&
(AuxData->PrivilegesUsed->PrivilegeCount))
{
/* Do the audit */
#if 0
@ -1656,7 +1656,7 @@ ObpCreateHandle(IN OB_OPEN_REASON OpenReason,
&AccessState->
SubjectSecurityContext,
GrantedAccess,
AuxData->PrivilegeSet,
AuxData->PrivilegesUsed,
TRUE,
ExGetPreviousMode());
#endif

20
ntoskrnl/se/access.c
View file

@ -23,10 +23,10 @@
* @param[in] Process
* Valid process object where subject context is to be captured.
*
* @param[in,out] AccessState
* @param[out] AccessState
* An initialized returned parameter to an access state.
*
* @param[in] AuxData
* @param[out] AuxData
* Auxiliary security data for access state.
*
* @param[in] Access
@ -43,8 +43,8 @@ NTAPI
SeCreateAccessStateEx(
_In_ PETHREAD Thread,
_In_ PEPROCESS Process,
_Inout_ PACCESS_STATE AccessState,
_In_ PAUX_ACCESS_DATA AuxData,
_Out_ PACCESS_STATE AccessState,
_Out_ __drv_aliasesMem PAUX_ACCESS_DATA AuxData,
_In_ ACCESS_MASK Access,
_In_ PGENERIC_MAPPING GenericMapping)
{
@ -88,7 +88,7 @@ SeCreateAccessStateEx(
}
/* Set the Auxiliary Data */
AuxData->PrivilegeSet = (PPRIVILEGE_SET)((ULONG_PTR)AccessState +
AuxData->PrivilegesUsed = (PPRIVILEGE_SET)((ULONG_PTR)AccessState +
FIELD_OFFSET(ACCESS_STATE,
Privileges));
if (GenericMapping) AuxData->GenericMapping = *GenericMapping;
@ -101,10 +101,10 @@ SeCreateAccessStateEx(
* @brief
* Creates an access state.
*
* @param[in,out] AccessState
* @param[out] AccessState
* An initialized returned parameter to an access state.
*
* @param[in] AuxData
* @param[out] AuxData
* Auxiliary security data for access state.
*
* @param[in] Access
@ -119,8 +119,8 @@ SeCreateAccessStateEx(
NTSTATUS
NTAPI
SeCreateAccessState(
_Inout_ PACCESS_STATE AccessState,
_In_ PAUX_ACCESS_DATA AuxData,
_Out_ PACCESS_STATE AccessState,
_Out_ __drv_aliasesMem PAUX_ACCESS_DATA AuxData,
_In_ ACCESS_MASK Access,
_In_ PGENERIC_MAPPING GenericMapping)
{
@ -158,7 +158,7 @@ SeDeleteAccessState(
/* Deallocate Privileges */
if (AccessState->PrivilegesAllocated)
ExFreePoolWithTag(AuxData->PrivilegeSet, TAG_PRIVILEGE_SET);
ExFreePoolWithTag(AuxData->PrivilegesUsed, TAG_PRIVILEGE_SET);
/* Deallocate Name and Type Name */
if (AccessState->ObjectName.Buffer)

14
ntoskrnl/se/priv.c
View file

@ -601,9 +601,9 @@ SeAppendPrivileges(
/* Calculate the size of the old privilege set */
OldPrivilegeSetSize = sizeof(PRIVILEGE_SET) +
(AuxData->PrivilegeSet->PrivilegeCount - 1) * sizeof(LUID_AND_ATTRIBUTES);
(AuxData->PrivilegesUsed->PrivilegeCount - 1) * sizeof(LUID_AND_ATTRIBUTES);
if (AuxData->PrivilegeSet->PrivilegeCount +
if (AuxData->PrivilegesUsed->PrivilegeCount +
Privileges->PrivilegeCount > INITIAL_PRIVILEGE_COUNT)
{
/* Calculate the size of the new privilege set */
@ -619,7 +619,7 @@ SeAppendPrivileges(
/* Copy original privileges from the acess state */
RtlCopyMemory(PrivilegeSet,
AuxData->PrivilegeSet,
AuxData->PrivilegesUsed,
OldPrivilegeSetSize);
/* Append privileges from the privilege set*/
@ -632,23 +632,23 @@ SeAppendPrivileges(
/* Free the old privilege set if it was allocated */
if (AccessState->PrivilegesAllocated != FALSE)
ExFreePoolWithTag(AuxData->PrivilegeSet, TAG_PRIVILEGE_SET);
ExFreePoolWithTag(AuxData->PrivilegesUsed, TAG_PRIVILEGE_SET);
/* Now we are using an allocated privilege set */
AccessState->PrivilegesAllocated = TRUE;
/* Assign the new privileges to the access state */
AuxData->PrivilegeSet = PrivilegeSet;
AuxData->PrivilegesUsed = PrivilegeSet;
}
else
{
/* Append privileges */
RtlCopyMemory((PVOID)((ULONG_PTR)AuxData->PrivilegeSet + OldPrivilegeSetSize),
RtlCopyMemory((PVOID)((ULONG_PTR)AuxData->PrivilegesUsed + OldPrivilegeSetSize),
(PVOID)((ULONG_PTR)Privileges + sizeof(PRIVILEGE_SET) - sizeof(LUID_AND_ATTRIBUTES)),
Privileges->PrivilegeCount * sizeof(LUID_AND_ATTRIBUTES));
/* Adjust the number of privileges in the target privilege set */
AuxData->PrivilegeSet->PrivilegeCount += Privileges->PrivilegeCount;
AuxData->PrivilegesUsed->PrivilegeCount += Privileges->PrivilegeCount;
}
return STATUS_SUCCESS;

19
sdk/include/ndk/setypes.h
View file

@ -255,9 +255,24 @@ typedef struct _TOKEN
typedef struct _AUX_ACCESS_DATA
{
PPRIVILEGE_SET PrivilegeSet;
PPRIVILEGE_SET PrivilegesUsed;
GENERIC_MAPPING GenericMapping;
ULONG Reserved;
ACCESS_MASK AccessesToAudit;
ACCESS_MASK MaximumAuditMask;
#if (NTDDI_VERSION >= NTDDI_LONGHORN)
GUID TransactionId;
#endif
#if (NTDDI_VERSION >= NTDDI_WIN7)
PVOID NewSecurityDescriptor;
PVOID ExistingSecurityDescriptor;
PVOID ParentSecurityDescriptor;
VOID (NTAPI *DerefSecurityDescriptor)(PVOID, PVOID);
PVOID SDLock;
ACCESS_REASONS AccessReasons;
#endif
#if (NTDDI_VERSION >= NTDDI_WIN8)
BOOLEAN GenerateStagingEvents;
#endif
} AUX_ACCESS_DATA, *PAUX_ACCESS_DATA;
//