From 94ea4f65f7864b6a584744800a0542203e50e0d9 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Sun, 18 Dec 2016 11:51:23 -0800 Subject: [PATCH] sys: FspFileNameIsValid, FspFileNameIsValidPattern: check path component length tst: memfs: allow filenames to be 512 chars long --- src/sys/create.c | 2 +- src/sys/dirctl.c | 4 +++- src/sys/driver.h | 5 +++-- src/sys/file.c | 4 +++- src/sys/fileinfo.c | 12 ++++++++++-- src/sys/name.c | 29 +++++++++++++++++++++++------ tst/memfs/memfs.cpp | 24 ++++++++++++++---------- 7 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/sys/create.c b/src/sys/create.c index b395b799..179a1184 100644 --- a/src/sys/create.c +++ b/src/sys/create.c @@ -299,7 +299,7 @@ static NTSTATUS FspFsvolCreateNoLock( ASSERT(NT_SUCCESS(Result)); /* check filename validity */ - if (!FspFileNameIsValid(&FileNode->FileName, + if (!FspFileNameIsValid(&FileNode->FileName, FsvolDeviceExtension->VolumeParams.MaxComponentLength, FsvolDeviceExtension->VolumeParams.NamedStreams ? &StreamPart : 0, &StreamType)) { diff --git a/src/sys/dirctl.c b/src/sys/dirctl.c index af5b2af0..3063f256 100644 --- a/src/sys/dirctl.c +++ b/src/sys/dirctl.c @@ -617,6 +617,7 @@ static NTSTATUS FspFsvolQueryDirectory( return STATUS_INVALID_DEVICE_REQUEST; NTSTATUS Result; + FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(FsvolDeviceObject); PFILE_OBJECT FileObject = IrpSp->FileObject; FSP_FILE_NODE *FileNode = FileObject->FsContext; FILE_INFORMATION_CLASS FileInformationClass = IrpSp->Parameters.QueryDirectory.FileInformationClass; @@ -636,7 +637,8 @@ static NTSTATUS FspFsvolQueryDirectory( return STATUS_INVALID_PARAMETER; /* check that FileName is valid (if supplied) */ - if (0 != FileName && !FspFileNameIsValidPattern(FileName)) + if (0 != FileName && + !FspFileNameIsValidPattern(FileName, FsvolDeviceExtension->VolumeParams.MaxComponentLength)) return STATUS_INVALID_PARAMETER; /* is this an allowed file information class? */ diff --git a/src/sys/driver.h b/src/sys/driver.h index 512173b0..8bf50b32 100644 --- a/src/sys/driver.h +++ b/src/sys/driver.h @@ -437,8 +437,9 @@ enum FspFileNameStreamTypeNone = 0, FspFileNameStreamTypeData = 1, }; -BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PULONG StreamType); -BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Pattern); +BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, ULONG MaxComponentLength, + PUNICODE_STRING StreamPart, PULONG StreamType); +BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Pattern, ULONG MaxComponentLength); VOID FspFileNameSuffix(PUNICODE_STRING Path, PUNICODE_STRING Remain, PUNICODE_STRING Suffix); #if 0 NTSTATUS FspFileNameUpcase( diff --git a/src/sys/file.c b/src/sys/file.c index fbbe911d..84ef3096 100644 --- a/src/sys/file.c +++ b/src/sys/file.c @@ -1911,7 +1911,9 @@ NTSTATUS FspMainFileOpen( PFILE_OBJECT MainFileObject; /* assert that the supplied name is actually a main file name */ - ASSERT(FspFileNameIsValid(MainFileName, 0, 0)); + ASSERT(FspFileNameIsValid(MainFileName, + FsvolDeviceExtension->VolumeParams.MaxComponentLength, + 0, 0)); *PMainFileHandle = 0; *PMainFileObject = 0; diff --git a/src/sys/fileinfo.c b/src/sys/fileinfo.c index f0a37681..06c2b190 100644 --- a/src/sys/fileinfo.c +++ b/src/sys/fileinfo.c @@ -1170,6 +1170,7 @@ static NTSTATUS FspFsvolSetRenameInformation( PAGED_CODE(); NTSTATUS Result; + FSP_FSVOL_DEVICE_EXTENSION *FsvolDeviceExtension = FspFsvolDeviceExtension(FsvolDeviceObject); PFILE_OBJECT FileObject = IrpSp->FileObject; PFILE_OBJECT TargetFileObject = IrpSp->Parameters.SetFile.FileObject; BOOLEAN ReplaceIfExists = IrpSp->Parameters.SetFile.ReplaceIfExists; @@ -1195,7 +1196,9 @@ static NTSTATUS FspFsvolSetRenameInformation( if (FileNode->IsRootDirectory) /* cannot rename root directory */ return STATUS_INVALID_PARAMETER; - if (!FspFileNameIsValid(&FileNode->FileName, 0, 0)) + if (!FspFileNameIsValid(&FileNode->FileName, + FsvolDeviceExtension->VolumeParams.MaxComponentLength, + 0, 0)) /* cannot rename streams (WinFsp limitation) */ return STATUS_INVALID_PARAMETER; @@ -1229,7 +1232,12 @@ retry: } Suffix.MaximumLength = Suffix.Length; - if (!FspFileNameIsValid(&Remain, 0, 0) || !FspFileNameIsValid(&Suffix, 0, 0)) + if (!FspFileNameIsValid(&Remain, + FsvolDeviceExtension->VolumeParams.MaxComponentLength, + 0, 0) || + !FspFileNameIsValid(&Suffix, + FsvolDeviceExtension->VolumeParams.MaxComponentLength, + 0, 0)) { /* cannot rename streams (WinFsp limitation) */ Result = STATUS_INVALID_PARAMETER; diff --git a/src/sys/name.c b/src/sys/name.c index 19649681..fed24cd4 100644 --- a/src/sys/name.c +++ b/src/sys/name.c @@ -17,8 +17,9 @@ #include -BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PULONG StreamType); -BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Pattern); +BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, ULONG MaxComponentLength, + PUNICODE_STRING StreamPart, PULONG StreamType); +BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Pattern, ULONG MaxComponentLength); VOID FspFileNameSuffix(PUNICODE_STRING Path, PUNICODE_STRING Remain, PUNICODE_STRING Suffix); NTSTATUS FspFileNameInExpression( PUNICODE_STRING Expression, @@ -34,7 +35,8 @@ NTSTATUS FspFileNameInExpression( #pragma alloc_text(PAGE, FspFileNameInExpression) #endif -BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PULONG StreamType) +BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, ULONG MaxComponentLength, + PUNICODE_STRING StreamPart, PULONG StreamType) { PAGED_CODE(); @@ -44,7 +46,7 @@ BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PUL if (0 == Path->Length || 0 != Path->Length % sizeof(WCHAR)) return FALSE; - PWSTR PathBgn, PathEnd, PathPtr, StreamTypeStr = 0; + PWSTR PathBgn, PathEnd, PathPtr, ComponentPtr, StreamTypeStr = 0; UCHAR Flags = FSRTL_NTFS_LEGAL; ULONG Colons = 0; WCHAR Char; @@ -52,6 +54,7 @@ BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PUL PathBgn = Path->Buffer; PathEnd = (PWSTR)((PUINT8)PathBgn + Path->Length); PathPtr = PathBgn; + ComponentPtr = PathPtr; while (PathEnd > PathPtr) { @@ -63,7 +66,12 @@ BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PUL if (0 < Colons) return FALSE; + /* path component cannot be longer than MaxComponentLength */ + if (PathPtr - ComponentPtr > MaxComponentLength) + return FALSE; + PathPtr++; + ComponentPtr = PathPtr; /* don't like multiple backslashes */ if (PathEnd > PathPtr && L'\\' == *PathPtr) @@ -106,6 +114,10 @@ BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PUL PathPtr++; } + /* path component cannot be longer than MaxComponentLength */ + if (PathPtr - ComponentPtr > MaxComponentLength) + return FALSE; + /* if we had no colons the path is valid */ if (0 == Colons) return TRUE; @@ -133,19 +145,20 @@ BOOLEAN FspFileNameIsValid(PUNICODE_STRING Path, PUNICODE_STRING StreamPart, PUL return FALSE; } -BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Path) +BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Path, ULONG MaxComponentLength) { PAGED_CODE(); if (0 != Path->Length % sizeof(WCHAR)) return FALSE; - PWSTR PathBgn, PathEnd, PathPtr; + PWSTR PathBgn, PathEnd, PathPtr, ComponentPtr; WCHAR Char; PathBgn = Path->Buffer; PathEnd = (PWSTR)((PUINT8)PathBgn + Path->Length); PathPtr = PathBgn; + ComponentPtr = PathPtr; while (PathEnd > PathPtr) { @@ -167,6 +180,10 @@ BOOLEAN FspFileNameIsValidPattern(PUNICODE_STRING Path) PathPtr++; } + /* path component cannot be longer than MaxComponentLength */ + if (PathPtr - ComponentPtr > MaxComponentLength) + return FALSE; + return TRUE; } diff --git a/tst/memfs/memfs.cpp b/tst/memfs/memfs.cpp index c5e5fdd0..91effce8 100644 --- a/tst/memfs/memfs.cpp +++ b/tst/memfs/memfs.cpp @@ -23,6 +23,10 @@ #include #include +#define MEMFS_MAX_PATH 512 +FSP_FSCTL_STATIC_ASSERT(MEMFS_MAX_PATH > MAX_PATH, + "MEMFS_MAX_PATH must be greater than MAX_PATH."); + /* * Define the MEMFS_NAME_NORMALIZATION macro to include name normalization support. */ @@ -177,7 +181,7 @@ BOOLEAN MemfsFileNameHasPrefix(PWSTR a, PWSTR b, BOOLEAN CaseInsensitive) typedef struct _MEMFS_FILE_NODE { - WCHAR FileName[MAX_PATH]; + WCHAR FileName[MEMFS_MAX_PATH]; FSP_FSCTL_FILE_INFO FileInfo; SIZE_T FileSecuritySize; PVOID FileSecurity; @@ -343,7 +347,7 @@ MEMFS_FILE_NODE *MemfsFileNodeMapGet(MEMFS_FILE_NODE_MAP *FileNodeMap, PWSTR Fil static inline MEMFS_FILE_NODE *MemfsFileNodeMapGetMain(MEMFS_FILE_NODE_MAP *FileNodeMap, PWSTR FileName0) { - WCHAR FileName[MAX_PATH]; + WCHAR FileName[MEMFS_MAX_PATH]; wcscpy_s(FileName, sizeof FileName / sizeof(WCHAR), FileName0); PWSTR StreamName = wcschr(FileName, L':'); if (0 == StreamName) @@ -362,7 +366,7 @@ MEMFS_FILE_NODE *MemfsFileNodeMapGetParent(MEMFS_FILE_NODE_MAP *FileNodeMap, PWS { WCHAR Root[2] = L"\\"; PWSTR Remain, Suffix; - WCHAR FileName[MAX_PATH]; + WCHAR FileName[MEMFS_MAX_PATH]; wcscpy_s(FileName, sizeof FileName / sizeof(WCHAR), FileName0); FspPathSuffix(FileName, &Remain, &Suffix, Root); MEMFS_FILE_NODE_MAP::iterator iter = FileNodeMap->find(Remain); @@ -437,11 +441,11 @@ BOOLEAN MemfsFileNodeMapEnumerateChildren(MEMFS_FILE_NODE_MAP *FileNodeMap, MEMF BOOLEAN IsDirectoryChild; if (0 != PrevFileName0) { - WCHAR PrevFileName[MAX_PATH]; + WCHAR PrevFileName[MEMFS_MAX_PATH]; size_t Length0 = wcslen(FileNode->FileName); size_t Length1 = 1 != Length0 || L'\\' != FileNode->FileName[0]; size_t Length2 = wcslen(PrevFileName0); - if (MAX_PATH <= Length0 + Length1 + Length2) + if (MEMFS_MAX_PATH <= Length0 + Length1 + Length2) /* fall back to linear scan! */ goto fallback; memcpy(PrevFileName, FileNode->FileName, Length0 * sizeof(WCHAR)); @@ -723,7 +727,7 @@ static NTSTATUS Create(FSP_FILE_SYSTEM *FileSystem, { MEMFS *Memfs = (MEMFS *)FileSystem->UserContext; #if defined(MEMFS_NAME_NORMALIZATION) - WCHAR FileNameBuf[MAX_PATH]; + WCHAR FileNameBuf[MEMFS_MAX_PATH]; #endif MEMFS_FILE_NODE *FileNode; MEMFS_FILE_NODE *ParentNode; @@ -731,7 +735,7 @@ static NTSTATUS Create(FSP_FILE_SYSTEM *FileSystem, NTSTATUS Result; BOOLEAN Inserted; - if (MAX_PATH <= wcslen(FileName)) + if (MEMFS_MAX_PATH <= wcslen(FileName)) return STATUS_OBJECT_NAME_INVALID; if (CreateOptions & FILE_DIRECTORY_FILE) @@ -765,7 +769,7 @@ static NTSTATUS Create(FSP_FILE_SYSTEM *FileSystem, RemainLength = wcslen(ParentNode->FileName); BSlashLength = 1 < RemainLength; SuffixLength = wcslen(Suffix); - if (MAX_PATH <= RemainLength + BSlashLength + SuffixLength) + if (MEMFS_MAX_PATH <= RemainLength + BSlashLength + SuffixLength) return STATUS_OBJECT_NAME_INVALID; memcpy(FileNameBuf, ParentNode->FileName, RemainLength * sizeof(WCHAR)); @@ -866,7 +870,7 @@ static NTSTATUS Open(FSP_FILE_SYSTEM *FileSystem, MEMFS_DIR_DESC *DirDesc = 0; NTSTATUS Result; - if (MAX_PATH <= wcslen(FileName)) + if (MEMFS_MAX_PATH <= wcslen(FileName)) return STATUS_OBJECT_NAME_INVALID; FileNode = MemfsFileNodeMapGet(Memfs->FileNodeMap, FileName); @@ -1216,7 +1220,7 @@ static NTSTATUS Rename(FSP_FILE_SYSTEM *FileSystem, for (Index = 0; Context.Count > Index; Index++) { DescendantFileNode = Context.FileNodes[Index]; - if (MAX_PATH <= wcslen(DescendantFileNode->FileName) - FileNameLen + NewFileNameLen) + if (MEMFS_MAX_PATH <= wcslen(DescendantFileNode->FileName) - FileNameLen + NewFileNameLen) { Result = STATUS_OBJECT_NAME_INVALID; goto exit;