[ros-diffs] [reactos] 01/02: [NTOS:SE] Make an access token effective after the end of token duplication

[George Bișoc]

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

commit a389f8aa0ce16db83922eb04af26a546fdcec265
Author:     George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Fri Jun 2 18:41:12 2023 +0200
Commit:     George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Sun Jun 4 11:09:22 2023 +0200

    [NTOS:SE] Make an access token effective after the end of token duplication
    
    Removing any disabled privileges or groups in the middle of token dynamic
    part allocation can pose problems. During the operation of making an access
    token as effective, we are toying with the privileges and groups arrays
    of the token.
    
    After that we are allocating the dynamic part and set EndMem (the end tail
    of the memory part) to that dynamic part, previously it was set to the
    variable part. As a matter of fact we are making the token effective in
    the middle where EndMem still points to VariablePart, thus DynamicPart
    will end up with memory pool blocks butchered in the pool list.
    
    Another problem, albeit not related to the DynamicPart corruption, is that
    the code starts iterating over the UserAndGroups array from 0, which is
    the actual user. One cannot simply remove the user from the array, so we
    have to start looping right from the groups.
    
    Move the token effective code part at the end of the SepDuplicateToken
    function, which fixes the random pool corruptions caused by the butchered
    DynamicPart.
    
    CORE-18986
---
 ntoskrnl/se/tokenlif.c | 86 ++++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 41 deletions(-)

diff --git a/ntoskrnl/se/tokenlif.c b/ntoskrnl/se/tokenlif.c
index 44f269ed40b..85a8ee01f78 100644
--- a/ntoskrnl/se/tokenlif.c
+++ b/ntoskrnl/se/tokenlif.c
@@ -665,6 +665,44 @@ SepDuplicateToken(
         }
     }
 
+    /* Now allocate the token's dynamic information area and set the data */
+    AccessToken->DynamicPart = ExAllocatePoolWithTag(PagedPool,
+                                                     DynamicPartSize,
+                                                     TAG_TOKEN_DYNAMIC);
+    if (AccessToken->DynamicPart == NULL)
+    {
+        Status = STATUS_INSUFFICIENT_RESOURCES;
+        goto Quit;
+    }
+
+    /* Unused memory in the dynamic area */
+    AccessToken->DynamicAvailable = 0;
+
+    /*
+     * Assign the primary group to the token
+     * and put it in the dynamic part as well.
+     */
+    EndMem = (PVOID)AccessToken->DynamicPart;
+    AccessToken->PrimaryGroup = EndMem;
+    RtlCopySid(RtlLengthSid(AccessToken->UserAndGroups[PrimaryGroupIndex].Sid),
+                            EndMem,
+                            AccessToken->UserAndGroups[PrimaryGroupIndex].Sid);
+    AccessToken->DefaultOwnerIndex = Token->DefaultOwnerIndex;
+    EndMem = (PVOID)((ULONG_PTR)EndMem +
RtlLengthSid(AccessToken->UserAndGroups[PrimaryGroupIndex].Sid));
+
+    /*
+     * The existing token has a default DACL only
+     * if it has an allocated dynamic part.
+     */
+    if (Token->DynamicPart && Token->DefaultDacl)
+    {
+        AccessToken->DefaultDacl = EndMem;
+
+        RtlCopyMemory(EndMem,
+                      Token->DefaultDacl,
+                      Token->DefaultDacl->AclSize);
+    }
+
     /*
      * Filter the token by removing the disabled privileges
      * and groups if the caller wants to duplicate an access
@@ -672,11 +710,15 @@ SepDuplicateToken(
      */
     if (EffectiveOnly)
     {
-        /* Begin querying the groups and search for disabled ones */
-        for (GroupsIndex = 0; GroupsIndex < AccessToken->UserAndGroupCount;
GroupsIndex++)
+        /*
+         * Begin querying the groups and search for disabled ones. Do not touch the
+         * user which is at the first position because it cannot be disabled, no
+         * matter what attributes it has.
+         */
+        for (GroupsIndex = 1; GroupsIndex < AccessToken->UserAndGroupCount;
GroupsIndex++)
         {
             /*
-             * A group or user is considered disabled if its attributes is either
+             * A group is considered disabled if its attributes is either
              * 0 or SE_GROUP_ENABLED is not included in the attributes flags list.
              * That is because a certain user and/or group can have several attributes
              * that bear no influence on whether a user/group is enabled or not
@@ -738,44 +780,6 @@ SepDuplicateToken(
         }
     }
 
-    /* Now allocate the token's dynamic information area and set the data */
-    AccessToken->DynamicPart = ExAllocatePoolWithTag(PagedPool,
-                                                     DynamicPartSize,
-                                                     TAG_TOKEN_DYNAMIC);
-    if (AccessToken->DynamicPart == NULL)
-    {
-        Status = STATUS_INSUFFICIENT_RESOURCES;
-        goto Quit;
-    }
-
-    /* Unused memory in the dynamic area */
-    AccessToken->DynamicAvailable = 0;
-
-    /*
-     * Assign the primary group to the token
-     * and put it in the dynamic part as well.
-     */
-    EndMem = (PVOID)AccessToken->DynamicPart;
-    AccessToken->PrimaryGroup = EndMem;
-    RtlCopySid(RtlLengthSid(AccessToken->UserAndGroups[PrimaryGroupIndex].Sid),
-                            EndMem,
-                            AccessToken->UserAndGroups[PrimaryGroupIndex].Sid);
-    AccessToken->DefaultOwnerIndex = Token->DefaultOwnerIndex;
-    EndMem = (PVOID)((ULONG_PTR)EndMem +
RtlLengthSid(AccessToken->UserAndGroups[PrimaryGroupIndex].Sid));
-
-    /*
-     * The existing token has a default DACL only
-     * if it has an allocated dynamic part.
-     */
-    if (Token->DynamicPart && Token->DefaultDacl)
-    {
-        AccessToken->DefaultDacl = EndMem;
-
-        RtlCopyMemory(EndMem,
-                      Token->DefaultDacl,
-                      Token->DefaultDacl->AclSize);
-    }
-
     /* Return the token to the caller */
     *NewAccessToken = AccessToken;
     Status = STATUS_SUCCESS;