https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2b2bdabe7221f9b35f135b013e44bce4bfd16c66
commit 2b2bdabe7221f9b35f135b013e44bce4bfd16c66
Author: Max Korostil <mrmk...@yandex.ru>
AuthorDate: Wed Feb 5 23:17:11 2025 +0300
Commit: GitHub <nore...@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'.')
