[ros-diffs] [reactos] 01/01: [NTOS:SE] Mark the token as no longer belonging to admin group upon effective duplication

[George Bișoc]

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

commit 0129de218b6263301ed141795a8e5e45f03d6b39
Author:     George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Mon Nov 1 20:35:21 2021 +0100
Commit:     George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Thu Nov 4 09:30:00 2021 +0100

    [NTOS:SE] Mark the token as no longer belonging to admin group upon effective
duplication
    
    A scenario where it happens that an access token belongs to an administrators group
but it's disabled (that is, SeAliasAdminsSid has no attributes or it doesn't have
SE_GROUP_ENABLED turn ON), the function removes this group from the token but still has
TOKEN_HAS_ADMIN_GROUP flag which can lead to erratic behavior across the kernel and
security modules -- implying that the token still belongs to administrators group.
    
    This is an oversight from my part.
---
 ntoskrnl/se/token.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c
index 061c907957a..51ba71f8e22 100644
--- a/ntoskrnl/se/token.c
+++ b/ntoskrnl/se/token.c
@@ -1196,6 +1196,20 @@ SepDuplicateToken(
             if (AccessToken->UserAndGroups[GroupsIndex].Attributes == 0 ||
                 (AccessToken->UserAndGroups[GroupsIndex].Attributes &
SE_GROUP_ENABLED) == 0)
             {
+                /*
+                 * If this group is an administrators group
+                 * and the token belongs to such group,
+                 * we've to take away TOKEN_HAS_ADMIN_GROUP
+                 * for the fact that's not enabled and as
+                 * such the token no longer belongs to
+                 * this group.
+                 */
+                if (RtlEqualSid(SeAliasAdminsSid,
+                                &AccessToken->UserAndGroups[GroupsIndex].Sid))
+                {
+                    AccessToken->TokenFlags &= ~TOKEN_HAS_ADMIN_GROUP;
+                }
+
                 /*
                  * A group is not enabled, it's time to remove
                  * from the token and update the groups index
@@ -1203,6 +1217,7 @@ SepDuplicateToken(
                  */
                 SepRemoveUserGroupToken(AccessToken, GroupsIndex);
                 GroupsIndex--;
+                continue;
             }
         }
 
@@ -1228,6 +1243,7 @@ SepDuplicateToken(
                  */
                 SepRemovePrivilegeToken(AccessToken, PrivilegesIndex);
                 PrivilegesIndex--;
+                continue;
             }
         }
     }