From 389a2da7ff94cfcefff3220797f61311519d6fa0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?=
 <hermes.belusca-maito@reactos.org>
Date: Thu, 19 May 2022 23:43:18 +0200
Subject: [PATCH] [NTOS:SE] SepCaptureAcl(): Add missing validation of the
 captured ACL (#4523)

- The ACL is however not validated when the function is called within
  kernel mode and no capture is actually being done.

- Simplify aspects of the function (returning early when possible).
---
 ntoskrnl/se/acl.c   | 89 ++++++++++++++++++++++++---------------------
 ntoskrnl/se/token.c |  2 +-
 2 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/ntoskrnl/se/acl.c b/ntoskrnl/se/acl.c
index e160b34c429..d7c53d4ac17 100644
--- a/ntoskrnl/se/acl.c
+++ b/ntoskrnl/se/acl.c
@@ -329,15 +329,15 @@ SepCreateImpersonationTokenDacl(
  *
  * @param[in] AccessMode
  * Processor level access mode. The processor mode determines how
- * are the input arguments probed.
+ * the input arguments are probed.
  *
  * @param[in] PoolType
  * Pool type for new captured ACL for creation. The pool type determines
- * how the ACL data should reside in the pool memory.
+ * in which memory pool the ACL data should reside.
  *
  * @param[in] CaptureIfKernel
- * If set to TRUE and the processor access mode being KernelMode, we're
- * capturing an ACL directly in the kernel. Otherwise we're capturing
+ * If set to TRUE and the processor access mode being KernelMode, we are
+ * capturing an ACL directly in the kernel. Otherwise we are capturing
  * within a kernel mode driver.
  *
  * @param[out] CapturedAcl
@@ -357,11 +357,19 @@ SepCaptureAcl(
     _Out_ PACL *CapturedAcl)
 {
     PACL NewAcl;
-    ULONG AclSize = 0;
-    NTSTATUS Status = STATUS_SUCCESS;
+    ULONG AclSize;
 
     PAGED_CODE();
 
+    /* If in kernel mode and we do not capture, just
+     * return the given ACL and don't validate it. */
+    if ((AccessMode == KernelMode) && !CaptureIfKernel)
+    {
+        *CapturedAcl = InputAcl;
+        return STATUS_SUCCESS;
+    }
+
+    /* Otherwise, capture and validate the ACL, depending on the access mode */
     if (AccessMode != KernelMode)
     {
         _SEH2_TRY
@@ -381,59 +389,56 @@ SepCaptureAcl(
         }
         _SEH2_END;
 
+        /* Validate the minimal size an ACL can have */
+        if (AclSize < sizeof(ACL))
+            return STATUS_INVALID_ACL;
+
         NewAcl = ExAllocatePoolWithTag(PoolType,
                                        AclSize,
                                        TAG_ACL);
-        if (NewAcl != NULL)
-        {
-            _SEH2_TRY
-            {
-                RtlCopyMemory(NewAcl,
-                              InputAcl,
-                              AclSize);
+        if (!NewAcl)
+            return STATUS_INSUFFICIENT_RESOURCES;
 
-                *CapturedAcl = NewAcl;
-            }
-            _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
-            {
-                /* Free the ACL and return the exception code */
-                ExFreePoolWithTag(NewAcl, TAG_ACL);
-                _SEH2_YIELD(return _SEH2_GetExceptionCode());
-            }
-            _SEH2_END;
-        }
-        else
+        _SEH2_TRY
         {
-            Status = STATUS_INSUFFICIENT_RESOURCES;
+            RtlCopyMemory(NewAcl, InputAcl, AclSize);
         }
-    }
-    else if (!CaptureIfKernel)
-    {
-        *CapturedAcl = InputAcl;
+        _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+        {
+            /* Free the ACL and return the exception code */
+            ExFreePoolWithTag(NewAcl, TAG_ACL);
+            _SEH2_YIELD(return _SEH2_GetExceptionCode());
+        }
+        _SEH2_END;
     }
     else
     {
         AclSize = InputAcl->AclSize;
 
+        /* Validate the minimal size an ACL can have */
+        if (AclSize < sizeof(ACL))
+            return STATUS_INVALID_ACL;
+
         NewAcl = ExAllocatePoolWithTag(PoolType,
                                        AclSize,
                                        TAG_ACL);
+        if (!NewAcl)
+            return STATUS_INSUFFICIENT_RESOURCES;
 
-        if (NewAcl != NULL)
-        {
-            RtlCopyMemory(NewAcl,
-                          InputAcl,
-                          AclSize);
-
-            *CapturedAcl = NewAcl;
-        }
-        else
-        {
-            Status = STATUS_INSUFFICIENT_RESOURCES;
-        }
+        RtlCopyMemory(NewAcl, InputAcl, AclSize);
     }
 
-    return Status;
+    /* Validate the captured ACL */
+    if (!RtlValidAcl(NewAcl))
+    {
+        /* Free the ACL and fail */
+        ExFreePoolWithTag(NewAcl, TAG_ACL);
+        return STATUS_INVALID_ACL;
+    }
+
+    /* It's valid, return it */
+    *CapturedAcl = NewAcl;
+    return STATUS_SUCCESS;
 }
 
 /**
diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c
index 1415e36009d..f215f4c1fe2 100644
--- a/ntoskrnl/se/token.c
+++ b/ntoskrnl/se/token.c
@@ -4518,7 +4518,7 @@ NtSetInformationToken(
                     {
                         PACL CapturedAcl;
 
-                        /* Capture and copy the dacl */
+                        /* Capture, validate, and copy the DACL */
                         Status = SepCaptureAcl(InputAcl,
                                                PreviousMode,
                                                PagedPool,