diff --git a/src/sys/dirctl.c b/src/sys/dirctl.c index 09b4d538..ab3fe685 100644 --- a/src/sys/dirctl.c +++ b/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 + - FSP_FSCTL_ALIGN_UP(BaseInfoLen + FileName.Length, sizeof(LONGLONG)) > DestBufEnd) + if ((PUINT8)DestBuf + BaseInfoLen + CopyLength > 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; - return STATUS_BUFFER_OVERFLOW; + /* copy as much of the file name as we can and 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 diff --git a/tst/winfsp-tests/dirctl-test.c b/tst/winfsp-tests/dirctl-test.c index 74b83013..bd6c31eb 100644 --- a/tst/winfsp-tests/dirctl-test.c +++ b/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); }