[ros-diffs] 03/03: [ADVAPI32] Improve a bit CreateProcessAsUser().

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=c446ce0d625b8c86e204b…

commit c446ce0d625b8c86e204b86e8e66503f10c8c580
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Fri Sep 28 00:36:59 2018 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Fri Sep 28 00:45:04 2018 +0200

    [ADVAPI32] Improve a bit CreateProcessAsUser().
    
    - Check whether the user-provided token is a primary token.
    
    - Do not fail when the RtlAdjustPrivilege() call fails (see the code
      comment for an explanation). TL;DR is: that call may indeed fail but
      the privilege may also not be necessary because the user-provided
      token is a restricted version of the caller's primary token.
      And this is situation is perfectly fine.
    
    This fixes Java 7 installation, CORE-14874.
---
 dll/win32/advapi32/misc/logon.c | 86 ++++++++++++++++++++++++++++++-----------
 1 file changed, 64 insertions(+), 22 deletions(-)

diff --git a/dll/win32/advapi32/misc/logon.c b/dll/win32/advapi32/misc/logon.c
index f4e3f851df..0b9d60320f 100644
--- a/dll/win32/advapi32/misc/logon.c
+++ b/dll/win32/advapi32/misc/logon.c
@@ -145,9 +145,30 @@ CreateProcessAsUserCommon(
 
     if (hToken != NULL)
     {
+        TOKEN_TYPE Type;
+        ULONG ReturnLength;
         OBJECT_ATTRIBUTES ObjectAttributes;
         HANDLE hTokenDup;
-        BOOLEAN PrivilegeSet = FALSE;
+        BOOLEAN PrivilegeSet = FALSE, HavePrivilege;
+
+        /* Check whether the user-provided token is a primary token */
+        // GetTokenInformation();
+        Status = NtQueryInformationToken(hToken,
+                                         TokenType,
+                                         &Type,
+                                         sizeof(Type),
+                                         &ReturnLength);
+        if (!NT_SUCCESS(Status))
+        {
+            ERR("NtQueryInformationToken() failed, Status 0x%08x\n", Status);
+            goto Quit;
+        }
+        if (Type != TokenPrimary)
+        {
+            ERR("Wrong token type for token 0x%p, expected TokenPrimary, got
%ld\n", hToken, Type);
+            Status = STATUS_BAD_TOKEN_TYPE;
+            goto Quit;
+        }
 
         /* Duplicate the token for this new process */
         InitializeObjectAttributes(&ObjectAttributes,
@@ -163,32 +184,43 @@ CreateProcessAsUserCommon(
                                   &hTokenDup);
         if (!NT_SUCCESS(Status))
         {
-            ERR("NtDuplicateToken failed, Status 0x%08x\n", Status);
-            TerminateProcess(lpProcessInformation->hProcess, Status);
-            SetLastError(RtlNtStatusToDosError(Status));
-            return FALSE;
+            ERR("NtDuplicateToken() failed, Status 0x%08x\n", Status);
+            goto Quit;
         }
 
         // FIXME: Do we always need SecurityImpersonation?
-        if (!ImpersonateSelf(SecurityImpersonation))
+        Status = RtlImpersonateSelf(SecurityImpersonation);
+        if (!NT_SUCCESS(Status))
         {
-            ERR("ImpersonateSelf(SecurityImpersonation) failed, last error:
%d\n", GetLastError());
+            ERR("RtlImpersonateSelf(SecurityImpersonation) failed, Status
0x%08x\n", Status);
             NtClose(hTokenDup);
-            TerminateProcess(lpProcessInformation->hProcess, RtlGetLastNtStatus());
-            // SetLastError(RtlNtStatusToDosError(Status));
-            return FALSE;
+            goto Quit;
         }
 
-        /* Acquire the process primary token assignment privilege */
-        Status = RtlAdjustPrivilege(SE_ASSIGNPRIMARYTOKEN_PRIVILEGE, TRUE, TRUE,
&PrivilegeSet);
-        if (!NT_SUCCESS(Status))
+        /*
+         * Attempt to acquire the process primary token assignment privilege
+         * in case we actually need it.
+         * The call will either succeed or fail when the caller has (or has not)
+         * enough rights.
+         * The last situation may not be dramatic for us. Indeed it may happen
+         * that the user-provided token is a restricted version of the caller's
+         * primary token (aka. a "child" token), or both tokens inherit (i.e.
are
+         * children, and are together "siblings") from a common parent token.
+         * In this case the NT kernel allows us to assign the token to the child
+         * process without the need for the assignment privilege, which is fine.
+         * On the contrary, if the user-provided token is completely arbitrary,
+         * then the NT kernel will enforce the presence of the assignment privilege:
+         * because we failed (by assumption) to assign the privilege, the process
+         * token assignment will fail as required. It is then the job of the
+         * caller to manually acquire the necessary privileges.
+         */
+        Status = RtlAdjustPrivilege(SE_ASSIGNPRIMARYTOKEN_PRIVILEGE,
+                                    TRUE, TRUE, &PrivilegeSet);
+        HavePrivilege = NT_SUCCESS(Status);
+        if (!HavePrivilege)
         {
-            ERR("RtlAdjustPrivilege(SE_ASSIGNPRIMARYTOKEN_PRIVILEGE) failed, Status
0x%08lx\n", Status);
-            RevertToSelf();
-            NtClose(hTokenDup);
-            TerminateProcess(lpProcessInformation->hProcess, Status);
-            SetLastError(RtlNtStatusToDosError(Status));
-            return FALSE;
+            ERR("RtlAdjustPrivilege(SE_ASSIGNPRIMARYTOKEN_PRIVILEGE) failed, Status
0x%08lx, "
+                "attempting to continue without it...\n", Status);
         }
 
         AccessToken.Token  = hTokenDup;
@@ -200,8 +232,12 @@ CreateProcessAsUserCommon(
                                          (PVOID)&AccessToken,
                                          sizeof(AccessToken));
 
-        /* Restore the privileges */
-        RtlAdjustPrivilege(SE_ASSIGNPRIMARYTOKEN_PRIVILEGE, PrivilegeSet, TRUE,
&PrivilegeSet);
+        /* Restore the privilege */
+        if (HavePrivilege)
+        {
+            RtlAdjustPrivilege(SE_ASSIGNPRIMARYTOKEN_PRIVILEGE,
+                               PrivilegeSet, TRUE, &PrivilegeSet);
+        }
 
         RevertToSelf();
 
@@ -211,7 +247,13 @@ CreateProcessAsUserCommon(
         /* Check whether NtSetInformationProcess() failed */
         if (!NT_SUCCESS(Status))
         {
-            ERR("NtSetInformationProcess failed, Status 0x%08x\n", Status);
+            ERR("NtSetInformationProcess() failed, Status 0x%08x\n", Status);
+            goto Quit;
+        }
+
+        if (!NT_SUCCESS(Status))
+        {
+Quit:
             TerminateProcess(lpProcessInformation->hProcess, Status);
             SetLastError(RtlNtStatusToDosError(Status));
             return FALSE;