[ros-diffs] [reactos] 06/07: [NTOS:SE] SepPerformTokenFiltering(): Fix corruption of DynamicPart (#4523)

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

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

commit 487d8601f9700a04f302a37c4fe399eab92da23e
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Fri May 20 02:33:00 2022 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Mon May 23 19:30:36 2022 +0200

    [NTOS:SE] SepPerformTokenFiltering(): Fix corruption of DynamicPart (#4523)
    
    The problem is that EndMem is changed to point to the DynamicPart of
    the token, but the code after that expects it to still point into the
    VariablePart instead.
    
    Problem fixed by moving the insertion of RestrictedSids much sooner
    (where the original ones are also being copied).
---
 ntoskrnl/se/token.c | 122 ++++++++++++++++++++++++++--------------------------
 1 file changed, 61 insertions(+), 61 deletions(-)

diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c
index 65018f60f3f..0c5296fe114 100644
--- a/ntoskrnl/se/token.c
+++ b/ntoskrnl/se/token.c
@@ -1277,8 +1277,8 @@ SepDuplicateToken(
             Status = STATUS_INSUFFICIENT_RESOURCES;
             goto Quit;
         }
-        EndMem = (PVOID)AccessToken->DynamicPart;
 
+        EndMem = (PVOID)AccessToken->DynamicPart;
         AccessToken->DefaultDacl = EndMem;
 
         RtlCopyMemory(AccessToken->DefaultDacl,
@@ -1973,8 +1973,8 @@ SepCreateToken(
             Status = STATUS_INSUFFICIENT_RESOURCES;
             goto Quit;
         }
-        EndMem = (PVOID)AccessToken->DynamicPart;
 
+        EndMem = (PVOID)AccessToken->DynamicPart;
         AccessToken->DefaultDacl = EndMem;
 
         RtlCopyMemory(AccessToken->DefaultDacl,
@@ -2306,6 +2306,64 @@ SepPerformTokenFiltering(
         }
     }
 
+    /*
+     * Insert the restricted SIDs into the token on
+     * the request by the caller.
+     */
+    if (RestrictedSidsIntoToken != NULL)
+    {
+        for (RestrictedSidsInList = 0; RestrictedSidsInList < RestrictedSidsCount;
RestrictedSidsInList++)
+        {
+            /* Did the caller assign attributes to the restricted SIDs? */
+            if (RestrictedSidsIntoToken[RestrictedSidsInList].Attributes != 0)
+            {
+                /* There mustn't be any attributes, bail out */
+                DPRINT1("SepPerformTokenFiltering(): There mustn't be any
attributes to restricted SIDs!\n");
+                Status = STATUS_INVALID_PARAMETER;
+                goto Quit;
+            }
+        }
+
+        /*
+         * Ensure that the token can hold the restricted SIDs
+         * (the variable length is calculated at the beginning
+         * of the routine call).
+         */
+        ASSERT(VariableLength >= RestrictedSidsLength);
+
+        /*
+         * Now let's begin inserting the restricted SIDs into the filtered
+         * access token from the list the caller gave us.
+         */
+        AccessToken->RestrictedSidCount = RestrictedSidsCount;
+        AccessToken->RestrictedSids = EndMem;
+        EndMem = (PVOID)((ULONG_PTR)EndMem + RestrictedSidsLength);
+        VariableLength -= RestrictedSidsLength;
+
+        RtlCopyMemory(AccessToken->RestrictedSids,
+                      RestrictedSidsIntoToken,
+                      AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES));
+
+        /*
+         * As we've copied the restricted SIDs into
+         * the token, we must assign them the following
+         * combination of attributes SE_GROUP_ENABLED,
+         * SE_GROUP_ENABLED_BY_DEFAULT and SE_GROUP_MANDATORY.
+         * With such attributes we estabilish that restricting
+         * SIDs into the token are enabled for access checks.
+         */
+        for (RestrictedSidsInToken = 0; RestrictedSidsInToken <
AccessToken->RestrictedSidCount; RestrictedSidsInToken++)
+        {
+            AccessToken->RestrictedSids[RestrictedSidsInToken].Attributes |=
(SE_GROUP_ENABLED | SE_GROUP_ENABLED_BY_DEFAULT | SE_GROUP_MANDATORY);
+        }
+
+        /*
+         * As we added restricted SIDs into the token, mark
+         * it as restricted.
+         */
+        AccessToken->TokenFlags |= TOKEN_IS_RESTRICTED;
+    }
+
     /* Search for the primary group */
     Status = SepFindPrimaryGroupAndDefaultOwner(AccessToken,
                                                 Token->PrimaryGroup,
@@ -2517,64 +2575,6 @@ SepPerformTokenFiltering(
         }
     }
 
-    /*
-     * Insert the restricted SIDs into the token on
-     * the request by the caller.
-     */
-    if (RestrictedSidsIntoToken != NULL)
-    {
-        for (RestrictedSidsInList = 0; RestrictedSidsInList < RestrictedSidsCount;
RestrictedSidsInList++)
-        {
-            /* Did the caller assign attributes to the restricted SIDs? */
-            if (RestrictedSidsIntoToken[RestrictedSidsInList].Attributes != 0)
-            {
-                /* There mustn't be any attributes, bail out */
-                DPRINT1("SepPerformTokenFiltering(): There mustn't be any
attributes to restricted SIDs!\n");
-                Status = STATUS_INVALID_PARAMETER;
-                goto Quit;
-            }
-        }
-
-        /*
-         * Ensure that the token can hold the restricted SIDs
-         * (the variable length is calculated at the beginning
-         * of the routine call).
-         */
-        ASSERT(VariableLength >= RestrictedSidsLength);
-
-        /*
-         * Now let's begin inserting the restricted SIDs into the filtered
-         * access token from the list the caller gave us.
-         */
-        AccessToken->RestrictedSidCount = RestrictedSidsCount;
-        AccessToken->RestrictedSids = EndMem;
-        EndMem = (PVOID)((ULONG_PTR)EndMem + RestrictedSidsLength);
-        VariableLength -= RestrictedSidsLength;
-
-        RtlCopyMemory(AccessToken->RestrictedSids,
-                      RestrictedSidsIntoToken,
-                      AccessToken->RestrictedSidCount * sizeof(SID_AND_ATTRIBUTES));
-
-        /*
-         * As we've copied the restricted SIDs into
-         * the token, we must assign them the following
-         * combination of attributes SE_GROUP_ENABLED,
-         * SE_GROUP_ENABLED_BY_DEFAULT and SE_GROUP_MANDATORY.
-         * With such attributes we estabilish that restricting
-         * SIDs into the token are enabled for access checks.
-         */
-        for (RestrictedSidsInToken = 0; RestrictedSidsInToken <
AccessToken->RestrictedSidCount; RestrictedSidsInToken++)
-        {
-            AccessToken->RestrictedSids[RestrictedSidsInToken].Attributes |=
(SE_GROUP_ENABLED | SE_GROUP_ENABLED_BY_DEFAULT | SE_GROUP_MANDATORY);
-        }
-
-        /*
-         * As we added restricted SIDs into the token, mark
-         * it as restricted.
-         */
-        AccessToken->TokenFlags |= TOKEN_IS_RESTRICTED;
-    }
-
     /* We've finally filtered the token, return it to the caller */
     *FilteredToken = AccessToken;
     Status = STATUS_SUCCESS;
@@ -5045,7 +5045,7 @@ SepAdjustGroups(
 
     PAGED_CODE();
 
-    /* Ensure that the token we get is not plain garbage */
+    /* Ensure that the token we get is valid */
     ASSERT(Token);
 
     /* Initialize the counters and begin the work */