From 4084448bd5b23b9528a7143c1d675ded5b8e3d1c Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Mon, 10 Oct 2016 17:29:16 -0700 Subject: [PATCH] sys,dll: properly implement stream create check --- inc/winfsp/fsctl.h | 3 ++- inc/winfsp/winfsp.h | 6 ++--- src/dll/fsop.c | 61 ++++++++++++++++++++++++++++++++++----------- src/dll/security.c | 34 ++++++++++++++++++++++++- src/sys/create.c | 6 ++++- 5 files changed, 90 insertions(+), 20 deletions(-) diff --git a/inc/winfsp/fsctl.h b/inc/winfsp/fsctl.h index b574288a..25948336 100644 --- a/inc/winfsp/fsctl.h +++ b/inc/winfsp/fsctl.h @@ -218,7 +218,8 @@ typedef struct UINT32 HasTraversePrivilege:1; /* requestor has TOKEN_HAS_TRAVERSE_PRIVILEGE */ UINT32 OpenTargetDirectory:1; /* open target dir and report FILE_{EXISTS,DOES_NOT_EXIST} */ UINT32 CaseSensitive:1; /* FileName comparisons should be case-sensitive */ - UINT32 NamedStream:1; /* request targets named stream; FileName has colon */ + UINT32 ReservedFlags:28; + UINT16 NamedStream; /* request targets named stream; colon offset in FileName */ } Create; struct { diff --git a/inc/winfsp/winfsp.h b/inc/winfsp/winfsp.h index 3b74e1df..adc9d213 100644 --- a/inc/winfsp/winfsp.h +++ b/inc/winfsp/winfsp.h @@ -1207,7 +1207,7 @@ FSP_API BOOLEAN FspFileSystemAddStreamInfo(FSP_FSCTL_STREAM_INFO *StreamInfo, FSP_API PGENERIC_MAPPING FspGetFileGenericMapping(VOID); FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, FSP_FSCTL_TRANSACT_REQ *Request, - BOOLEAN CheckParentDirectory, BOOLEAN AllowTraverseCheck, + BOOLEAN CheckParentOrMain, BOOLEAN AllowTraverseCheck, UINT32 DesiredAccess, PUINT32 PGrantedAccess/* or ReparsePointIndex */, PSECURITY_DESCRIPTOR *PSecurityDescriptor); FSP_API NTSTATUS FspCreateSecurityDescriptor(FSP_FILE_SYSTEM *FileSystem, @@ -1223,11 +1223,11 @@ FSP_API VOID FspDeleteSecurityDescriptor(PSECURITY_DESCRIPTOR SecurityDescriptor static inline NTSTATUS FspAccessCheck(FSP_FILE_SYSTEM *FileSystem, FSP_FSCTL_TRANSACT_REQ *Request, - BOOLEAN CheckParentDirectory, BOOLEAN AllowTraverseCheck, + BOOLEAN CheckParentOrMain, BOOLEAN AllowTraverseCheck, UINT32 DesiredAccess, PUINT32 PGrantedAccess) { return FspAccessCheckEx(FileSystem, Request, - CheckParentDirectory, AllowTraverseCheck, + CheckParentOrMain, AllowTraverseCheck, DesiredAccess, PGrantedAccess, 0); } diff --git a/src/dll/fsop.c b/src/dll/fsop.c index 27376423..a7aac2b0 100644 --- a/src/dll/fsop.c +++ b/src/dll/fsop.c @@ -140,24 +140,57 @@ NTSTATUS FspFileSystemCreateCheck(FSP_FILE_SYSTEM *FileSystem, UINT32 GrantedAccess; /* - * CreateCheck consists of checking the parent directory for the - * FILE_ADD_SUBDIRECTORY or FILE_ADD_FILE rights (depending on whether - * we are creating a file or directory). + * CreateCheck does different checks depending on whether we are + * creating a new file/directory or a new stream. * - * If the access check succeeds and MAXIMUM_ALLOWED has been requested - * then we go ahead and grant all access to the creator. + * - CreateCheck for a new file consists of checking the parent directory + * for the FILE_ADD_SUBDIRECTORY or FILE_ADD_FILE rights (depending on + * whether we are creating a file or directory). + * + * If the access check succeeds and MAXIMUM_ALLOWED has been requested + * then we go ahead and grant all access to the creator. + * + * - CreateCheck for a new stream consists of checking the main file for + * FILE_WRITE_DATA access, unless FILE_DELETE_ON_CLOSE is requested in + * which case we also check for DELETE access. + * + * If the access check succeeds and MAXIMUM_ALLOWED was not requested + * then we reset the DELETE and FILE_WRITE_DATA accesses based on whether + * they were actually requested in DesiredAccess. */ - Result = FspAccessCheckEx(FileSystem, Request, TRUE, AllowTraverseCheck, - (Request->Req.Create.CreateOptions & FILE_DIRECTORY_FILE) ? - FILE_ADD_SUBDIRECTORY : FILE_ADD_FILE, - &GrantedAccess, PSecurityDescriptor); - if (STATUS_REPARSE == Result) - Result = FspFileSystemCallResolveReparsePoints(FileSystem, Request, Response, GrantedAccess); - else if (NT_SUCCESS(Result)) + if (!Request->Req.Create.NamedStream) { - *PGrantedAccess = (MAXIMUM_ALLOWED & Request->Req.Create.DesiredAccess) ? - FspGetFileGenericMapping()->GenericAll : Request->Req.Create.DesiredAccess; + Result = FspAccessCheckEx(FileSystem, Request, TRUE, AllowTraverseCheck, + (Request->Req.Create.CreateOptions & FILE_DIRECTORY_FILE) ? + FILE_ADD_SUBDIRECTORY : FILE_ADD_FILE, + &GrantedAccess, PSecurityDescriptor); + if (STATUS_REPARSE == Result) + Result = FspFileSystemCallResolveReparsePoints(FileSystem, Request, Response, GrantedAccess); + else if (NT_SUCCESS(Result)) + { + *PGrantedAccess = (MAXIMUM_ALLOWED & Request->Req.Create.DesiredAccess) ? + FspGetFileGenericMapping()->GenericAll : Request->Req.Create.DesiredAccess; + } + } + else + { + *PSecurityDescriptor = 0; + + Result = FspAccessCheckEx(FileSystem, Request, TRUE, AllowTraverseCheck, + Request->Req.Create.DesiredAccess | + FILE_WRITE_DATA | + ((Request->Req.Create.CreateOptions & FILE_DELETE_ON_CLOSE) ? DELETE : 0), + &GrantedAccess, 0); + if (STATUS_REPARSE == Result) + Result = FspFileSystemCallResolveReparsePoints(FileSystem, Request, Response, GrantedAccess); + else if (NT_SUCCESS(Result)) + { + *PGrantedAccess = GrantedAccess; + if (0 == (Request->Req.Create.DesiredAccess & MAXIMUM_ALLOWED)) + *PGrantedAccess &= ~(DELETE | FILE_WRITE_DATA) | + (Request->Req.Create.DesiredAccess & (DELETE | FILE_WRITE_DATA)); + } } return Result; diff --git a/src/dll/security.c b/src/dll/security.c index 7d100533..fd0fdc29 100644 --- a/src/dll/security.c +++ b/src/dll/security.c @@ -63,10 +63,21 @@ static inline ULONG FspPathSuffixIndex(PWSTR FileName) FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, FSP_FSCTL_TRANSACT_REQ *Request, - BOOLEAN CheckParentDirectory, BOOLEAN AllowTraverseCheck, + BOOLEAN CheckParentOrMain, BOOLEAN AllowTraverseCheck, UINT32 DesiredAccess, PUINT32 PGrantedAccess, PSECURITY_DESCRIPTOR *PSecurityDescriptor) { + BOOLEAN CheckParentDirectory, CheckMainFile; + + CheckParentDirectory = CheckMainFile = FALSE; + if (CheckParentOrMain) + { + if (!Request->Req.Create.NamedStream) + CheckParentDirectory = TRUE; + else + CheckMainFile = TRUE; + } + *PGrantedAccess = 0; if (0 != PSecurityDescriptor) *PSecurityDescriptor = 0; @@ -100,6 +111,11 @@ FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, if (CheckParentDirectory) FspPathSuffix((PWSTR)Request->Buffer, &FileName, &Suffix, Root); + else if (CheckMainFile) + { + ((PWSTR)Request->Buffer)[Request->Req.Create.NamedStream / sizeof(WCHAR)] = L'\0'; + FileName = (PWSTR)Request->Buffer; + } else FileName = (PWSTR)Request->Buffer; @@ -269,6 +285,20 @@ FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, goto exit; } } + else if (CheckMainFile) + { + /* + * We check to see if this is a reparse point and FILE_OPEN_REPARSE_POINT + * was not specified, in which case we return STATUS_REPARSE. + */ + if (0 != (FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && + 0 == (Request->Req.Create.CreateOptions & FILE_OPEN_REPARSE_POINT)) + { + FileAttributes = FspPathSuffixIndex(FileName); + Result = STATUS_REPARSE; + goto exit; + } + } else { /* @@ -344,6 +374,8 @@ exit: if (STATUS_OBJECT_NAME_NOT_FOUND == Result) Result = STATUS_OBJECT_PATH_NOT_FOUND; } + else if (CheckMainFile) + ((PWSTR)Request->Buffer)[Request->Req.Create.NamedStream / sizeof(WCHAR)] = L':'; return Result; } diff --git a/src/sys/create.c b/src/sys/create.c index 3d79078a..a3639bf3 100644 --- a/src/sys/create.c +++ b/src/sys/create.c @@ -477,7 +477,11 @@ static NTSTATUS FspFsvolCreateNoLock( Request->Req.Create.HasTraversePrivilege = HasTraversePrivilege; Request->Req.Create.OpenTargetDirectory = BooleanFlagOn(Flags, SL_OPEN_TARGET_DIRECTORY); Request->Req.Create.CaseSensitive = CaseSensitiveRequested; - Request->Req.Create.NamedStream = 0 != StreamPart.Length; + Request->Req.Create.NamedStream = MainFileName.Length; + + ASSERT( + 0 == StreamPart.Length && 0 == MainFileName.Length || + 0 != StreamPart.Length && 0 != MainFileName.Length); /* copy the security descriptor (if any) into the request */ if (0 != SecurityDescriptorSize)