[reactos] 01/01: [RTL][NTDLL_APITEST] Fix buffer overflow in RtlDosSearchPath_Ustr (#7698)
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2b2bdabe7221f9b35f135b...
commit 2b2bdabe7221f9b35f135b013e44bce4bfd16c66 Author: Max Korostil mrmks04@yandex.ru AuthorDate: Wed Feb 5 23:17:11 2025 +0300 Commit: GitHub noreply@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'.')
-
Max Korostil