sys: IRP_MN_QUERY_DIRECTORY: correctly handle STATUS_BUFFER_OVERFLOW cases

src/sys/dirctl.c

@@ -100,7 +100,7 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
         __VA_ARGS__\
         Info = DestBuf;\
         RtlCopyMemory(Info, &InfoStruct, FIELD_OFFSET(TYPE, FileName));\
-        RtlCopyMemory(Info->FileName, DirInfo->FileNameBuf, FileName.Length);\
+        RtlCopyMemory(Info->FileName, DirInfo->FileNameBuf, CopyLength);\
     } while (0,0)
 #define FILL_INFO(TYPE, ...)\
     FILL_INFO_BASE(TYPE,\
@@ -117,6 +117,7 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
 
     PAGED_CODE();
 
+    NTSTATUS Result = STATUS_SUCCESS;
     BOOLEAN MatchAll = FspFileDescDirectoryPatternMatchAll == DirectoryPattern->Buffer;
     BOOLEAN Loop = TRUE, DirectoryOffsetFound = FALSE;
     FSP_FSCTL_DIR_INFO *DirInfo = *PDirInfo;
@@ -124,7 +125,7 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
     PUINT8 DestBufBgn = (PUINT8)DestBuf;
     PUINT8 DestBufEnd = (PUINT8)DestBuf + *PDestLen;
     PVOID PrevDestBuf = 0;
-    ULONG BaseInfoLen;
+    ULONG BaseInfoLen, CopyLength;
     UNICODE_STRING FileName;
 
     *PDestLen = 0;
@@ -178,17 +179,27 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
             FileName.MaximumLength = (USHORT)(DirInfoSize - sizeof(FSP_FSCTL_DIR_INFO));
             FileName.Buffer = DirInfo->FileNameBuf;
 
+            /* CopyLength is the same as FileName.Length except on STATUS_BUFFER_OVERFLOW */
+            CopyLength = FileName.Length;
+
             if (MatchAll || FsRtlIsNameInExpression(DirectoryPattern, &FileName, CaseInsensitive, 0))
             {
-                if ((PUINT8)DestBuf +
+                if ((PUINT8)DestBuf + BaseInfoLen + CopyLength > DestBufEnd)
-                    FSP_FSCTL_ALIGN_UP(BaseInfoLen + FileName.Length, sizeof(LONGLONG)) > DestBufEnd)
                 {
-                    if (0 == *PDestLen)
+                    /* if we have already copied something exit the loop */
+                    if (0 != *PDestLen)
+                        break;
+
+                    if ((PUINT8)DestBuf + BaseInfoLen > DestBufEnd)
+                        /* buffer is too small, can't copy anything */
+                        return STATUS_BUFFER_TOO_SMALL;
+                    else
                     {
-                        *PDestLen = BaseInfoLen + FileName.Length;
+                        /* copy as much of the file name as we can and return STATUS_BUFFER_OVERFLOW */
-                        return STATUS_BUFFER_OVERFLOW;
+                        CopyLength = (USHORT)(DestBufEnd - ((PUINT8)DestBuf + BaseInfoLen));
+                        Result = STATUS_BUFFER_OVERFLOW;
+                        Loop = FALSE;
                     }
-                    break;
                 }
 
                 if (0 != PrevDestBuf)
@@ -196,7 +207,7 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
                 PrevDestBuf = DestBuf;
 
                 *PDirectoryOffset = DirInfo->NextOffset;
-                *PDestLen = (ULONG)((PUINT8)DestBuf + BaseInfoLen + FileName.Length - DestBufBgn);
+                *PDestLen = (ULONG)((PUINT8)DestBuf + BaseInfoLen + CopyLength - DestBufBgn);
 
                 switch (FileInformationClass)
                 {
@@ -238,7 +249,7 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
                 }
 
                 DestBuf = (PVOID)((PUINT8)DestBuf +
-                    FSP_FSCTL_ALIGN_UP(BaseInfoLen + FileName.Length, sizeof(LONGLONG)));
+                    FSP_FSCTL_ALIGN_UP(BaseInfoLen + CopyLength, sizeof(LONGLONG)));
 
                 if (ReturnSingleEntry)
                     /* cannot just break, *PDirInfo must be advanced */
@@ -250,15 +261,18 @@ static NTSTATUS FspFsvolQueryDirectoryCopy(
     }
     except (EXCEPTION_EXECUTE_HANDLER)
     {
-        NTSTATUS Result = GetExceptionCode();
+        Result = GetExceptionCode();
         if (STATUS_INSUFFICIENT_RESOURCES == Result)
             return Result;
         return FsRtlIsNtstatusExpected(Result) ? STATUS_INVALID_USER_BUFFER : Result;
     }
 
+    /* our code flow should allow only these two status codes here */
+    ASSERT(STATUS_SUCCESS == Result || STATUS_BUFFER_OVERFLOW == Result);
+
     *PDirInfo = DirInfo;
 
-    return STATUS_SUCCESS;
+    return Result;
 
 #undef FILL_INFO
 #undef FILL_INFO_BASE

tst/winfsp-tests/dirctl-test.c

@@ -226,6 +226,142 @@ void querydir_expire_cache_test(void)
     }
 }
 
+typedef struct _FILE_DIRECTORY_INFORMATION {
+    ULONG NextEntryOffset;
+    ULONG FileIndex;
+    LARGE_INTEGER CreationTime;
+    LARGE_INTEGER LastAccessTime;
+    LARGE_INTEGER LastWriteTime;
+    LARGE_INTEGER ChangeTime;
+    LARGE_INTEGER EndOfFile;
+    LARGE_INTEGER AllocationSize;
+    ULONG FileAttributes;
+    ULONG FileNameLength;
+    WCHAR FileName[1];
+} FILE_DIRECTORY_INFORMATION, *PFILE_DIRECTORY_INFORMATION;
+
+NTSTATUS
+NTAPI
+NtQueryDirectoryFile (
+    _In_ HANDLE FileHandle,
+    _In_opt_ HANDLE Event,
+    _In_opt_ PIO_APC_ROUTINE ApcRoutine,
+    _In_opt_ PVOID ApcContext,
+    _Out_ PIO_STATUS_BLOCK IoStatusBlock,
+    _Out_writes_bytes_(Length) PVOID FileInformation,
+    _In_ ULONG Length,
+    _In_ FILE_INFORMATION_CLASS FileInformationClass,
+    _In_ BOOLEAN ReturnSingleEntry,
+    _In_opt_ PUNICODE_STRING FileName,
+    _In_ BOOLEAN RestartScan
+    );
+
+static void querydir_buffer_overflow_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout, ULONG SleepTimeout)
+{
+    void *memfs = memfs_start_ex(Flags, FileInfoTimeout);
+
+    HANDLE Handle;
+    BOOL Success;
+    NTSTATUS Status;
+    IO_STATUS_BLOCK IoStatus;
+    WCHAR FilePath[MAX_PATH];
+    FILE_DIRECTORY_INFORMATION DirInfo;
+    UNICODE_STRING Pattern;
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\file0",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+
+    Handle = CreateFileW(FilePath,
+        GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, 0, CREATE_NEW,
+        FILE_ATTRIBUTE_NORMAL, 0);
+    ASSERT(INVALID_HANDLE_VALUE != Handle);
+    CloseHandle(Handle);
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+
+    Handle = CreateFileW(FilePath,
+        GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_EXISTING,
+        FILE_FLAG_BACKUP_SEMANTICS, 0);
+    ASSERT(INVALID_HANDLE_VALUE != Handle);
+
+    Pattern.Length = Pattern.MaximumLength = sizeof L"file0" - sizeof(WCHAR);
+    Pattern.Buffer = L"file0";
+
+    memset(&DirInfo, 0, sizeof DirInfo);
+    Status = NtQueryDirectoryFile(
+        Handle,
+        0, 0, 0,
+        &IoStatus,
+        &DirInfo,
+        0,
+        FileDirectoryInformation,
+        FALSE,
+        &Pattern,
+        FALSE);
+    ASSERT(STATUS_INFO_LENGTH_MISMATCH == Status);
+
+    memset(&DirInfo, 0, sizeof DirInfo);
+    Status = NtQueryDirectoryFile(
+        Handle,
+        0, 0, 0,
+        &IoStatus,
+        &DirInfo,
+        sizeof(FILE_DIRECTORY_INFORMATION),
+        FileDirectoryInformation,
+        FALSE,
+        &Pattern,
+        FALSE);
+    ASSERT(STATUS_BUFFER_OVERFLOW == Status);
+    ASSERT(sizeof(FILE_DIRECTORY_INFORMATION) == IoStatus.Information);
+    ASSERT(Pattern.Length == DirInfo.FileNameLength);
+
+    memset(&DirInfo, 0, sizeof DirInfo);
+    Status = NtQueryDirectoryFile(
+        Handle,
+        0, 0, 0,
+        &IoStatus,
+        &DirInfo,
+        sizeof(FILE_DIRECTORY_INFORMATION),
+        FileDirectoryInformation,
+        TRUE,
+        &Pattern,
+        FALSE);
+    ASSERT(STATUS_BUFFER_OVERFLOW == Status);
+    ASSERT(sizeof(FILE_DIRECTORY_INFORMATION) == IoStatus.Information);
+    ASSERT(Pattern.Length == DirInfo.FileNameLength);
+
+    CloseHandle(Handle);
+
+    StringCbPrintfW(FilePath, sizeof FilePath, L"%s%s\\file0",
+        Prefix ? L"" : L"\\\\?\\GLOBALROOT", Prefix ? Prefix : memfs_volumename(memfs));
+
+    Success = DeleteFileW(FilePath);
+    ASSERT(Success);
+
+    memfs_stop(memfs);
+}
+
+void querydir_buffer_overflow_test(void)
+{
+    if (NtfsTests)
+    {
+        WCHAR DirBuf[MAX_PATH] = L"\\\\?\\";
+        GetCurrentDirectoryW(MAX_PATH - 4, DirBuf + 4);
+        querydir_buffer_overflow_dotest(-1, DirBuf, 0, 0);
+    }
+    if (WinFspDiskTests)
+    {
+        querydir_buffer_overflow_dotest(MemfsDisk, 0, 0, 0);
+        querydir_buffer_overflow_dotest(MemfsDisk, 0, 1000, 0);
+    }
+    if (WinFspNetTests)
+    {
+        querydir_buffer_overflow_dotest(MemfsNet, L"\\\\memfs\\share", 0, 0);
+        querydir_buffer_overflow_dotest(MemfsNet, L"\\\\memfs\\share", 1000, 0);
+    }
+}
+
 static unsigned __stdcall dirnotify_dotest_thread(void *FilePath)
 {
     FspDebugLog(__FUNCTION__ ": \"%S\"\n", FilePath);
@@ -332,5 +468,6 @@ void dirctl_tests(void)
 {
     TEST(querydir_test);
     TEST(querydir_expire_cache_test);
+    TEST(querydir_buffer_overflow_test);
     TEST(dirnotify_test);
 }