https://git.reactos.org/?p=reactos.git;a=commitdiff;h=5912c1165073b8d1015ad1e0f0d229a7c50a181d
commit 5912c1165073b8d1015ad1e0f0d229a7c50a181d Author: George Bișoc <[email protected]> AuthorDate: Fri Jun 18 18:38:12 2021 +0200 Commit: George Bișoc <[email protected]> CommitDate: Fri Jun 18 18:38:12 2021 +0200 [NTOS:SE] Minor refactor on NtOpenThreadTokenEx - Remove a redundant call of ObReferenceObjectByHandle. Not only it didn't make much sense (we reference the object from thread handle and the new thread object referencing the same handle!), specifying a request access of THREAD_ALL_ACCESS for the thread object is kind of suspicious and all of these access rights are unwanted. - Add some failure checks involving the CopyOnOpen code paths - Add some DPRINT1 debug prints (concerning the CopyOnOpen code paths as usual) --- ntoskrnl/se/token.c | 75 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index 74ec4142917..507a8d73bad 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -4233,7 +4233,7 @@ NtOpenThreadTokenEx(IN HANDLE ThreadHandle, IN ULONG HandleAttributes, OUT PHANDLE TokenHandle) { - PETHREAD Thread, NewThread; + PETHREAD Thread; HANDLE hToken; PTOKEN Token, NewToken = NULL, PrimaryToken; BOOLEAN CopyOnOpen, EffectiveOnly; @@ -4307,40 +4307,53 @@ NtOpenThreadTokenEx(IN HANDLE ThreadHandle, if (CopyOnOpen) { - Status = ObReferenceObjectByHandle(ThreadHandle, THREAD_ALL_ACCESS, - PsThreadType, KernelMode, - (PVOID*)&NewThread, NULL); - if (NT_SUCCESS(Status)) - { - PrimaryToken = PsReferencePrimaryToken(NewThread->ThreadsProcess); + PrimaryToken = PsReferencePrimaryToken(Thread->ThreadsProcess); - Status = SepCreateImpersonationTokenDacl(Token, PrimaryToken, &Dacl); + Status = SepCreateImpersonationTokenDacl(Token, PrimaryToken, &Dacl); - ObFastDereferenceObject(&NewThread->ThreadsProcess->Token, PrimaryToken); + ObFastDereferenceObject(&Thread->ThreadsProcess->Token, PrimaryToken); - if (NT_SUCCESS(Status)) + if (NT_SUCCESS(Status)) + { + if (Dacl) { - if (Dacl) + Status = RtlCreateSecurityDescriptor(&SecurityDescriptor, + SECURITY_DESCRIPTOR_REVISION); + if (!NT_SUCCESS(Status)) { - RtlCreateSecurityDescriptor(&SecurityDescriptor, - SECURITY_DESCRIPTOR_REVISION); - RtlSetDaclSecurityDescriptor(&SecurityDescriptor, TRUE, Dacl, - FALSE); + DPRINT1("NtOpenThreadTokenEx(): Failed to create a security descriptor (Status 0x%lx)\n", Status); } - InitializeObjectAttributes(&ObjectAttributes, NULL, HandleAttributes, - NULL, Dacl ? &SecurityDescriptor : NULL); - - Status = SepDuplicateToken(Token, &ObjectAttributes, EffectiveOnly, - TokenImpersonation, ImpersonationLevel, - KernelMode, &NewToken); - if (NT_SUCCESS(Status)) + Status = RtlSetDaclSecurityDescriptor(&SecurityDescriptor, TRUE, Dacl, + FALSE); + if (!NT_SUCCESS(Status)) { - ObReferenceObject(NewToken); - Status = ObInsertObject(NewToken, NULL, DesiredAccess, 0, NULL, - &hToken); + DPRINT1("NtOpenThreadTokenEx(): Failed to set a DACL to the security descriptor (Status 0x%lx)\n", Status); } } + + InitializeObjectAttributes(&ObjectAttributes, NULL, HandleAttributes, + NULL, Dacl ? &SecurityDescriptor : NULL); + + Status = SepDuplicateToken(Token, &ObjectAttributes, EffectiveOnly, + TokenImpersonation, ImpersonationLevel, + KernelMode, &NewToken); + if (!NT_SUCCESS(Status)) + { + DPRINT1("NtOpenThreadTokenEx(): Failed to duplicate the token (Status 0x%lx)\n"); + } + + ObReferenceObject(NewToken); + Status = ObInsertObject(NewToken, NULL, DesiredAccess, 0, NULL, + &hToken); + if (!NT_SUCCESS(Status)) + { + DPRINT1("NtOpenThreadTokenEx(): Failed to insert the token object (Status 0x%lx)\n", Status); + } + } + else + { + DPRINT1("NtOpenThreadTokenEx(): Failed to impersonate token from DACL (Status 0x%lx)\n", Status); } } else @@ -4348,6 +4361,10 @@ NtOpenThreadTokenEx(IN HANDLE ThreadHandle, Status = ObOpenObjectByPointer(Token, HandleAttributes, NULL, DesiredAccess, SeTokenObjectType, PreviousMode, &hToken); + if (!NT_SUCCESS(Status)) + { + DPRINT1("NtOpenThreadTokenEx(): Failed to open the object (Status 0x%lx)\n", Status); + } } if (Dacl) ExFreePoolWithTag(Dacl, TAG_ACL); @@ -4361,13 +4378,15 @@ NtOpenThreadTokenEx(IN HANDLE ThreadHandle, if (NT_SUCCESS(Status) && CopyOnOpen) { - PsImpersonateClient(Thread, NewToken, FALSE, EffectiveOnly, ImpersonationLevel); + Status = PsImpersonateClient(Thread, NewToken, FALSE, EffectiveOnly, ImpersonationLevel); + if (!NT_SUCCESS(Status)) + { + DPRINT1("NtOpenThreadTokenEx(): Failed to impersonate the client (Status 0x%lx)\n"); + } } if (NewToken) ObDereferenceObject(NewToken); - if (CopyOnOpen && NewThread) ObDereferenceObject(NewThread); - ObDereferenceObject(Thread); if (NT_SUCCESS(Status))
