[ros-diffs] [reactos] 02/02: [NTOS:LPC] Ensure debug-traced pointer-given user-mode data is captured. (#4399)

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

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

commit b3c55b9e6c9b0f62142f7dbe3e3059f3f81c7f0d
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Sun Mar 20 22:55:06 2022 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Wed May 17 17:40:37 2023 +0200

    [NTOS:LPC] Ensure debug-traced pointer-given user-mode data is captured. (#4399)
    
    CORE-18098
---
 ntoskrnl/lpc/connect.c | 64 ++++++++++++++++++++++++++++++++++----------------
 ntoskrnl/lpc/create.c  | 21 ++++++++++++-----
 ntoskrnl/lpc/send.c    | 27 +++++++++++----------
 3 files changed, 74 insertions(+), 38 deletions(-)

diff --git a/ntoskrnl/lpc/connect.c b/ntoskrnl/lpc/connect.c
index d1f2426ef73..69dff778ab4 100644
--- a/ntoskrnl/lpc/connect.c
+++ b/ntoskrnl/lpc/connect.c
@@ -90,6 +90,9 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
     NTSTATUS Status = STATUS_SUCCESS;
     KPROCESSOR_MODE PreviousMode = KeGetPreviousMode();
     PETHREAD Thread = PsGetCurrentThread();
+#if DBG
+    UNICODE_STRING CapturedPortName;
+#endif
     SECURITY_QUALITY_OF_SERVICE CapturedQos;
     PORT_VIEW CapturedClientView;
     PSID CapturedServerSid;
@@ -105,13 +108,6 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
     PTOKEN_USER TokenUserInfo;
 
     PAGED_CODE();
-    LPCTRACE(LPC_CONNECT_DEBUG,
-             "Name: %wZ. SecurityQos: %p. Views: %p/%p. Sid: %p\n",
-             PortName,
-             SecurityQos,
-             ClientView,
-             ServerView,
-             ServerSid);
 
     /* Check if the call comes from user mode */
     if (PreviousMode != KernelMode)
@@ -141,7 +137,6 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
                     /* Invalid size */
                     _SEH2_YIELD(return STATUS_INVALID_PARAMETER);
                 }
-
             }
 
             /* Capture the server view */
@@ -172,7 +167,7 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
                 ProbeForWrite(ConnectionInformation, ConnectionInfoLength,
sizeof(ULONG));
 
             CapturedServerSid = ServerSid;
-            if (ServerSid != NULL)
+            if (ServerSid)
             {
                 /* Capture it */
                 Status = SepCaptureSid(ServerSid,
@@ -231,6 +226,21 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
         CapturedServerSid = ServerSid;
     }
 
+#if DBG
+    /* Capture the port name for DPRINT only - ObReferenceObjectByName does
+     * its own capture. As it is used only for debugging, ignore any failure;
+     * the string is zeroed out in such case. */
+    ProbeAndCaptureUnicodeString(&CapturedPortName, PreviousMode, PortName);
+
+    LPCTRACE(LPC_CONNECT_DEBUG,
+             "Name: %wZ. SecurityQos: %p. Views: %p/%p. Sid: %p\n",
+             &CapturedPortName,
+             SecurityQos,
+             ClientView,
+             ServerView,
+             ServerSid);
+#endif
+
     /* Get the port */
     Status = ObReferenceObjectByName(PortName,
                                      0,
@@ -242,7 +252,10 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
                                      (PVOID*)&Port);
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("Failed to reference port '%wZ': 0x%lx\n", PortName,
Status);
+#if DBG
+        DPRINT1("Failed to reference port '%wZ': 0x%lx\n",
&CapturedPortName, Status);
+        ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode);
+#endif
 
         if (CapturedServerSid != ServerSid)
             SepReleaseSid(CapturedServerSid, PreviousMode, TRUE);
@@ -253,7 +266,10 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
     /* This has to be a connection port */
     if ((Port->Flags & LPCP_PORT_TYPE_MASK) != LPCP_CONNECTION_PORT)
     {
-        DPRINT1("Port '%wZ' is not a connection port (Flags: 0x%lx)\n",
PortName, Port->Flags);
+#if DBG
+        DPRINT1("Port '%wZ' is not a connection port (Flags: 0x%lx)\n",
&CapturedPortName, Port->Flags);
+        ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode);
+#endif
 
         /* It isn't, so fail */
         ObDereferenceObject(Port);
@@ -282,7 +298,9 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
                 if (!RtlEqualSid(CapturedServerSid, TokenUserInfo->User.Sid))
                 {
                     /* Fail */
-                    DPRINT1("Port '%wZ': server SID mismatch\n",
PortName);
+#if DBG
+                    DPRINT1("Port '%wZ': server SID mismatch\n",
&CapturedPortName);
+#endif
                     Status = STATUS_SERVER_SID_MISMATCH;
                 }
 
@@ -293,21 +311,27 @@ NtSecureConnectPort(OUT PHANDLE PortHandle,
         else
         {
             /* Invalid SID */
-            DPRINT1("Port '%wZ': server SID mismatch\n", PortName);
+#if DBG
+            DPRINT1("Port '%wZ': server SID mismatch\n",
&CapturedPortName);
+#endif
             Status = STATUS_SERVER_SID_MISMATCH;
         }
 
         /* Finally release the captured SID, we don't need it anymore */
         if (CapturedServerSid != ServerSid)
             SepReleaseSid(CapturedServerSid, PreviousMode, TRUE);
+    }
 
-        /* Check if SID failed */
-        if (!NT_SUCCESS(Status))
-        {
-            /* Quit */
-            ObDereferenceObject(Port);
-            return Status;
-        }
+#if DBG
+    ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode);
+#endif
+
+    /* Check if SID failed */
+    if (ServerSid && !NT_SUCCESS(Status))
+    {
+        /* Quit */
+        ObDereferenceObject(Port);
+        return Status;
     }
 
     /* Create the client port */
diff --git a/ntoskrnl/lpc/create.c b/ntoskrnl/lpc/create.c
index aaa5ed70518..7c017b6b7e1 100644
--- a/ntoskrnl/lpc/create.c
+++ b/ntoskrnl/lpc/create.c
@@ -50,11 +50,13 @@ LpcpCreatePort(OUT PHANDLE PortHandle,
     NTSTATUS Status;
     KPROCESSOR_MODE PreviousMode = KeGetPreviousMode();
     UNICODE_STRING CapturedObjectName, *ObjectName;
+#if DBG
+    UNICODE_STRING CapturedPortName;
+#endif
     PLPCP_PORT_OBJECT Port;
     HANDLE Handle;
 
     PAGED_CODE();
-    LPCTRACE(LPC_CREATE_DEBUG, "Name: %wZ\n",
ObjectAttributes->ObjectName);
 
     RtlInitEmptyUnicodeString(&CapturedObjectName, NULL, 0);
 
@@ -70,10 +72,7 @@ LpcpCreatePort(OUT PHANDLE PortHandle,
             ProbeForRead(ObjectAttributes, sizeof(*ObjectAttributes), sizeof(ULONG));
             ObjectName = ((volatile
OBJECT_ATTRIBUTES*)ObjectAttributes)->ObjectName;
             if (ObjectName)
-            {
-                ProbeForRead(ObjectName, sizeof(*ObjectName), 1);
-                CapturedObjectName = *(volatile UNICODE_STRING*)ObjectName;
-            }
+                CapturedObjectName = ProbeForReadUnicodeString(ObjectName);
         }
         _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
         {
@@ -88,10 +87,20 @@ LpcpCreatePort(OUT PHANDLE PortHandle,
             CapturedObjectName = *(ObjectAttributes->ObjectName);
     }
 
-    /* Normalize the buffer pointer in case we don't have a name */
+    /* Normalize the buffer pointer in case we don't have
+     * a name, for initializing an unconnected port. */
     if (CapturedObjectName.Length == 0)
         CapturedObjectName.Buffer = NULL;
 
+#if DBG
+    /* Capture the port name for DPRINT only - ObCreateObject does its
+     * own capture. As it is used only for debugging, ignore any failure;
+     * the string is zeroed out in such case. */
+    ProbeAndCaptureUnicodeString(&CapturedPortName, PreviousMode,
&CapturedObjectName);
+    LPCTRACE(LPC_CREATE_DEBUG, "Name: %wZ\n", &CapturedPortName);
+    ReleaseCapturedUnicodeString(&CapturedPortName, PreviousMode);
+#endif
+
     /* Create the Object */
     Status = ObCreateObject(PreviousMode,
                             LpcPortObjectType,
diff --git a/ntoskrnl/lpc/send.c b/ntoskrnl/lpc/send.c
index d6b22063014..317ad1e4563 100644
--- a/ntoskrnl/lpc/send.c
+++ b/ntoskrnl/lpc/send.c
@@ -449,11 +449,6 @@ NtRequestPort(IN HANDLE PortHandle,
     PLPCP_MESSAGE Message;
 
     PAGED_CODE();
-    LPCTRACE(LPC_SEND_DEBUG,
-             "Handle: %p. Message: %p. Type: %lx\n",
-             PortHandle,
-             LpcRequest,
-             LpcpGetMessageType(LpcRequest));
 
     /* Check if the call comes from user mode */
     if (PreviousMode != KernelMode)
@@ -476,6 +471,12 @@ NtRequestPort(IN HANDLE PortHandle,
         CapturedLpcRequest = *LpcRequest;
     }
 
+    LPCTRACE(LPC_SEND_DEBUG,
+             "Handle: %p. Message: %p. Type: %lx\n",
+             PortHandle,
+             LpcRequest,
+             LpcpGetMessageType(&CapturedLpcRequest));
+
     /* Get the message type */
     MessageType = CapturedLpcRequest.u2.s2.Type | LPC_DATAGRAM;
 
@@ -709,15 +710,10 @@ NtRequestWaitReplyPort(IN HANDLE PortHandle,
     PLPCP_DATA_INFO DataInfo;
 
     PAGED_CODE();
-    LPCTRACE(LPC_SEND_DEBUG,
-             "Handle: %p. Messages: %p/%p. Type: %lx\n",
-             PortHandle,
-             LpcRequest,
-             LpcReply,
-             LpcpGetMessageType(LpcRequest));
 
     /* Check if the thread is dying */
-    if (Thread->LpcExitThreadCalled) return STATUS_THREAD_IS_TERMINATING;
+    if (Thread->LpcExitThreadCalled)
+        return STATUS_THREAD_IS_TERMINATING;
 
     /* Check for user mode access */
     if (PreviousMode != KernelMode)
@@ -757,6 +753,13 @@ NtRequestWaitReplyPort(IN HANDLE PortHandle,
         }
     }
 
+    LPCTRACE(LPC_SEND_DEBUG,
+             "Handle: %p. Messages: %p/%p. Type: %lx\n",
+             PortHandle,
+             LpcRequest,
+             LpcReply,
+             LpcpGetMessageType(&CapturedLpcRequest));
+
     /* This flag is undocumented. Remove it before continuing */
     CapturedLpcRequest.u2.s2.Type &= ~0x4000;