Description
I identified 4 issues in the FsRtlIsNameInExpressionPrivate function.
- Overflow of the BackTrackingBuffer stack buffer by 2 bytes.
Triggered by input: Expression="FI<<<<<<<<", Name="FILE"
Bug found with afl-fuzz.
Patch:diff --git a/ntoskrnl/fsrtl/name.c b/ntoskrnl/fsrtl/name.c
index fef798cc86d..5ad84ada7bf 100644
--- a/ntoskrnl/fsrtl/name.c
+++ b/ntoskrnl/fsrtl/name.c
@@ -135,7 +135,7 @@ FsRtlIsNameInExpressionPrivate(IN PUNICODE_STRING Expression,
}
/* If buffer too small */
- if (BackTrackingPosition > BackTrackingBufferSize - 2)
+ if (BackTrackingPosition > BackTrackingBufferSize - 3)
{
/* We should only ever get here once! */
ASSERT(AllocatedBuffer == NULL);
- Logic error that can result in inadvertently swapping the contents of BackTracking and OldBackTracking.
Triggered by input: Expression="<<<<<<<<<.<", Name="."
Bug found by myself, triggering input found with afl-fuzz.
Patch:diff --git a/ntoskrnl/fsrtl/name.c b/ntoskrnl/fsrtl/name.c
index fef798cc86d..37da9531ddd 100644
--- a/ntoskrnl/fsrtl/name.c
+++ b/ntoskrnl/fsrtl/name.c
@@ -157,21 +157,21 @@ FsRtlIsNameInExpressionPrivate(IN PUNICODE_STRING Expression,
goto Exit;
}
- /* Backtracking is at the start of the buffer */
- BackTracking = AllocatedBuffer;
-
- /* Copy BackTrackingBuffer content */
- RtlCopyMemory(BackTracking,
- BackTrackingBuffer,
+ /* Copy BackTracking content. Note that it can point to either BackTrackingBuffer or OldBackTrackingBuffer */
+ RtlCopyMemory(AllocatedBuffer,
+ BackTracking,
RTL_NUMBER_OF(BackTrackingBuffer) * sizeof(USHORT));
- /* OldBackTracking is after BackTracking */
- OldBackTracking = &BackTracking[BackTrackingBufferSize];
+ /* Place current Backtracking is at the start of the new buffer */
+ BackTracking = AllocatedBuffer;
- /* Copy OldBackTrackingBuffer content */
- RtlCopyMemory(OldBackTracking,
- OldBackTrackingBuffer,
+ /* Copy OldBackTracking content */
+ RtlCopyMemory(&BackTracking[BackTrackingBufferSize],
+ OldBackTracking,
RTL_NUMBER_OF(OldBackTrackingBuffer) * sizeof(USHORT));
+
+ /* Place current OldBackTracking after current BackTracking in the buffer */
+ OldBackTracking = &BackTracking[BackTrackingBufferSize];
}
/* Basic check to test if chars are equal */
- Incorrect handling of DOS_STAR due to a logic bug. It causes DontSkipDot to be always set to FALSE whenever the for loop is entered, effectively rendering DOS_STAR equivalent to regular star (*).
Triggered by input: Expression="F<", Name="FILE.TXT"
Patch:diff --git a/ntoskrnl/fsrtl/name.c b/ntoskrnl/fsrtl/name.c
index fef798cc86d..8bc14da7cfc 100644
--- a/ntoskrnl/fsrtl/name.c
+++ b/ntoskrnl/fsrtl/name.c
@@ -200,7 +200,7 @@ FsRtlIsNameInExpressionPrivate(IN PUNICODE_STRING Expression,
DontSkipDot = TRUE;
if (!EndOfName && Name->Buffer[NamePosition] == '.')
{
- for (Position = NamePosition - 1; Position < Name->Length; Position++)
+ for (Position = NamePosition + 1; Position < Name->Length / sizeof(WCHAR); Position++)
{
if (Name->Buffer[Position] == L'.')
{
- Excessive buffer allocation by a factor of 2. The following loop exit condition should make it clear that BackTrackingBufferSize need not be larger than Expression->Length + 1:
if (ExpressionPosition == Expression->Length)
{
BackTracking[BackTrackingPosition++] = Expression->Length * 2;
break;
}
Patch:
diff --git a/ntoskrnl/fsrtl/name.c b/ntoskrnl/fsrtl/name.c
index fef798cc86d..a6f0c004b7f 100644
--- a/ntoskrnl/fsrtl/name.c
+++ b/ntoskrnl/fsrtl/name.c
@@ -142,9 +142,9 @@ FsRtlIsNameInExpressionPrivate(IN PUNICODE_STRING Expression,
ASSERT((BackTracking == BackTrackingBuffer) || (BackTracking == OldBackTrackingBuffer));
ASSERT((OldBackTracking == BackTrackingBuffer) || (OldBackTracking == OldBackTrackingBuffer));
/* Calculate buffer size */
- BackTrackingBufferSize = (Expression->Length + 1) * 2;
+ BackTrackingBufferSize = Expression->Length + 1;
/* Allocate memory for both back-tracking buffers */
AllocatedBuffer = ExAllocatePoolWithTag(PagedPool | POOL_RAISE_IF_ALLOCATION_FAILURE,
2 * BackTrackingBufferSize * sizeof(USHORT),