[ros-diffs] [reactos] 02/02: [NTDLL:CSR] Perform more thorough validation of the parameters in CsrAllocateCaptureBuffer().

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

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

commit d86301f72bb562b89c8dfa63781b764742317f8b
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Mon Oct 5 02:15:14 2020 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Mon Oct 5 02:22:45 2020 +0200

    [NTDLL:CSR] Perform more thorough validation of the parameters in
CsrAllocateCaptureBuffer().
    
    Complements commit 7e2db773.
    
    - Validate the argument count.
    
    - Validate the total buffer size: the total size of the header plus
      the pointer-offset array and the provided buffer, together with
      the alignment padding for each argument, must be less than MAXLONG
      aligned to 4-byte boundary.
---
 dll/ntdll/csr/capture.c | 35 ++++++++++++++++++++++++++++-------
 dll/ntdll/csr/connect.c |  2 +-
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/dll/ntdll/csr/capture.c b/dll/ntdll/csr/capture.c
index 759f74bcdf9..1a388d467a4 100644
--- a/dll/ntdll/csr/capture.c
+++ b/dll/ntdll/csr/capture.c
@@ -91,13 +91,35 @@ CsrAllocateCaptureBuffer(IN ULONG ArgumentCount,
                          IN ULONG BufferSize)
 {
     PCSR_CAPTURE_BUFFER CaptureBuffer;
+    ULONG OffsetsArraySize;
+    ULONG MaximumSize;
 
-    /* Validate size */
-    if (BufferSize >= MAXLONG) return NULL;
+    /* Validate the argument count. Note that on server side, CSRSRV
+     * limits the count to MAXUSHORT; here we are a bit more lenient. */
+    if (ArgumentCount > (MAXLONG / sizeof(ULONG_PTR)))
+        return NULL;
+
+    OffsetsArraySize = ArgumentCount * sizeof(ULONG_PTR);
+
+    /*
+     * Validate the total buffer size.
+     * The total size of the header plus the pointer-offset array and the
+     * provided buffer, together with the alignment padding for each argument,
+     * must be less than MAXLONG aligned to 4-byte boundary.
+     */
+    MaximumSize = (MAXLONG & ~3) - FIELD_OFFSET(CSR_CAPTURE_BUFFER,
PointerOffsetsArray);
+    if (OffsetsArraySize >= MaximumSize)
+        return NULL;
+    MaximumSize -= OffsetsArraySize;
+    if (BufferSize >= MaximumSize)
+        return NULL;
+    MaximumSize -= BufferSize;
+    if ((ArgumentCount * 3) + 3 >= MaximumSize)
+        return NULL;
 
     /* Add the size of the header and of the pointer-offset array */
     BufferSize += FIELD_OFFSET(CSR_CAPTURE_BUFFER, PointerOffsetsArray) +
-                    (ArgumentCount * sizeof(ULONG_PTR));
+                    OffsetsArraySize;
 
     /* Add the size of the alignment padding for each argument */
     BufferSize += ArgumentCount * 3;
@@ -113,13 +135,12 @@ CsrAllocateCaptureBuffer(IN ULONG ArgumentCount,
     CaptureBuffer->Size = BufferSize;
     CaptureBuffer->PointerCount = 0;
 
-    /* Initialize all the offsets */
-    RtlZeroMemory(CaptureBuffer->PointerOffsetsArray,
-                  ArgumentCount * sizeof(ULONG_PTR));
+    /* Initialize the pointer-offset array */
+    RtlZeroMemory(CaptureBuffer->PointerOffsetsArray, OffsetsArraySize);
 
     /* Point to the start of the free buffer */
     CaptureBuffer->BufferEnd =
(PVOID)((ULONG_PTR)CaptureBuffer->PointerOffsetsArray +
-                                       ArgumentCount * sizeof(ULONG_PTR));
+                                       OffsetsArraySize);
 
     /* Return the address of the buffer */
     return CaptureBuffer;
diff --git a/dll/ntdll/csr/connect.c b/dll/ntdll/csr/connect.c
index 213b48b9457..f9d007b701f 100644
--- a/dll/ntdll/csr/connect.c
+++ b/dll/ntdll/csr/connect.c
@@ -407,7 +407,7 @@ CsrClientCallServer(IN OUT PCSR_API_MESSAGE ApiMessage,
             ApiMessage->CsrCaptureData = (PCSR_CAPTURE_BUFFER)
                 ((ULONG_PTR)CaptureBuffer + CsrPortMemoryDelta);
 
-            /* Lock the buffer. */
+            /* Lock the buffer */
             CaptureBuffer->BufferEnd = NULL;
 
             /*