[ros-diffs] [reactos] 01/01: [RTL][NTDLL_APITEST] Fix buffer overflow in RtlDosSearchPath_Ustr (#7698)

[Max Korostil]

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

commit 2b2bdabe7221f9b35f135b013e44bce4bfd16c66
Author:     Max Korostil <mrmks04(a)yandex.ru>
AuthorDate: Wed Feb 5 23:17:11 2025 +0300
Commit:     GitHub <noreply(a)github.com>
CommitDate: Wed Feb 5 21:17:11 2025 +0100

    [RTL][NTDLL_APITEST] Fix buffer overflow in RtlDosSearchPath_Ustr (#7698)
    
    In addition:
    - `IS_PATH_SEPARATOR(*--End)` -> `--End; IS_PATH_SEPARATOR(*End)` fix,
    - Use SIZE_T type for `WorstCaseLength` and `NamePlusExtLength`.
---
 .../apitests/ntdll/RtlDosSearchPath_Ustr.c         | 19 ++++++++++++
 sdk/lib/rtl/path.c                                 | 35 ++++++++++++----------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/modules/rostests/apitests/ntdll/RtlDosSearchPath_Ustr.c
b/modules/rostests/apitests/ntdll/RtlDosSearchPath_Ustr.c
index 29794bf4e8d..e6eb386cc29 100644
--- a/modules/rostests/apitests/ntdll/RtlDosSearchPath_Ustr.c
+++ b/modules/rostests/apitests/ntdll/RtlDosSearchPath_Ustr.c
@@ -210,4 +210,23 @@ START_TEST(RtlDosSearchPath_Ustr)
     ok_eq_pointer(FullNameOut, NULL);
     ok_eq_ulong(FilePartSize, 0UL);
     ok_eq_ulong(LengthNeeded, 0UL);
+
+    /* Buffer overflow test
+     * length(longDirName) + length(longFileName) + length(ext) = MAX_PATH
+     */
+    RtlInitUnicodeString(&PathString, L"C:\\Program
Files\\Very_long_test_path_which_can_trigger_heap_overflow_test_1234567890______________________________________________________AB");
+    RtlInitUnicodeString(&FileNameString,
L"this_is_long_file_name_for_checking______________________________________________________________________________CD");
+    RtlInitUnicodeString(&ExtensionString, L".txt");
+    StartSeh()
+        Status = RtlDosSearchPath_Ustr(0,
+                                       &PathString,
+                                       &FileNameString,
+                                       &ExtensionString,
+                                       &CallerBuffer,
+                                       &DynamicString,
+                                       &FullNameOut,
+                                       &FilePartSize,
+                                       &LengthNeeded);
+        ok_eq_hex(Status, STATUS_NO_SUCH_FILE);
+    EndSeh(STATUS_SUCCESS);
 }
diff --git a/sdk/lib/rtl/path.c b/sdk/lib/rtl/path.c
index e045e16ebb4..4104daeb838 100644
--- a/sdk/lib/rtl/path.c
+++ b/sdk/lib/rtl/path.c
@@ -2571,8 +2571,8 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
     NTSTATUS Status;
     RTL_PATH_TYPE PathType;
     PWCHAR p, End, CandidateEnd, SegmentEnd;
-    SIZE_T SegmentSize, ByteCount, PathSize, MaxPathSize = 0;
-    USHORT NamePlusExtLength, WorstCaseLength, ExtensionLength = 0;
+    SIZE_T WorstCaseLength, NamePlusExtLength, SegmentSize, ByteCount, PathSize,
MaxPathSize = 0;
+    USHORT ExtensionLength = 0;
     PUNICODE_STRING FullIsolatedPath;
     DPRINT("DOS Path Search: %lx %wZ %wZ %wZ %wZ %wZ\n",
             Flags, PathString, FileNameString, ExtensionString, CallerBuffer,
DynamicString);
@@ -2669,7 +2669,8 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
                 while (End > FileNameString->Buffer)
                 {
                     /* If we find a path separator, there's no extension */
-                    if (IS_PATH_SEPARATOR(*--End)) break;
+                    --End;
+                    if (IS_PATH_SEPARATOR(*End)) break;
 
                     /* Otherwise, did we find an extension dot? */
                     if (*End == L'.')
@@ -2689,17 +2690,20 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
             /* Start parsing the path name, looking for path separators */
             End = &PathString->Buffer[PathString->Length / sizeof(WCHAR)];
             p = End;
-            while ((p > PathString->Buffer) && (*--p == L';'))
+            while (p > PathString->Buffer)
             {
-                /* This is the size of the path -- handle a trailing slash */
-                PathSize = End - p - 1;
-                if ((PathSize) && !(IS_PATH_SEPARATOR(*(End - 1)))) PathSize++;
+                if (*--p == L';')
+                {
+                    /* This is the size of the path -- handle a trailing slash */
+                    PathSize = End - p - 1;
+                    if ((PathSize) && !(IS_PATH_SEPARATOR(*(End - 1))))
PathSize++;
 
-                /* Check if we found a bigger path than before */
-                if (PathSize > MaxPathSize) MaxPathSize = PathSize;
+                    /* Check if we found a bigger path than before */
+                    if (PathSize > MaxPathSize) MaxPathSize = PathSize;
 
-                /* Keep going with the path after this path separator */
-                End = p;
+                    /* Keep going with the path after this path separator */
+                    End = p;
+                }
             }
 
             /* This is the trailing path, run the same code as above */
@@ -2749,7 +2753,7 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
 
             /* Now check if our initial static buffer is too small */
             if (StaticCandidateString.MaximumLength <
-                (SegmentSize + ExtensionLength + FileNameString->Length))
+                (SegmentSize + ExtensionLength + FileNameString->Length +
sizeof(UNICODE_NULL)))
             {
                 /* At this point we should've been using our static buffer */
                 ASSERT(StaticCandidateString.Buffer == StaticCandidateBuffer);
@@ -2784,8 +2788,7 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
                 }
 
                 /* Now allocate the dynamic string */
-                StaticCandidateString.MaximumLength = FileNameString->Length +
-                                                      WorstCaseLength;
+                StaticCandidateString.MaximumLength = WorstCaseLength;
                 StaticCandidateString.Buffer = RtlpAllocateStringMemory(WorstCaseLength,
                                                                         TAG_USTR);
                 if (!StaticCandidateString.Buffer)
@@ -2832,6 +2835,7 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
             StaticCandidateString.Length = (USHORT)(CandidateEnd -
                                             StaticCandidateString.Buffer) *
                                            sizeof(WCHAR);
+            ASSERT(StaticCandidateString.Length <
StaticCandidateString.MaximumLength);
 
             /* Check if this file exists */
             DPRINT("BUFFER: %S\n", StaticCandidateString.Buffer);
@@ -2901,7 +2905,8 @@ RtlDosSearchPath_Ustr(IN ULONG Flags,
                 while (End > p)
                 {
                     /* If there's a path separator, there's no extension */
-                    if (IS_PATH_SEPARATOR(*--End)) break;
+                    --End;
+                    if (IS_PATH_SEPARATOR(*End)) break;
 
                     /* Othwerwise, did we find an extension dot? */
                     if (*End == L'.')