From f642ea57be8220aab844f0a4f197025891cdca36 Mon Sep 17 00:00:00 2001 From: Bill Zissimopoulos Date: Wed, 19 Oct 2016 11:54:22 -0700 Subject: [PATCH] dll: FspAccessCheckEx: test access checks without traverse privilege --- src/dll/security.c | 26 +++++++++++++++++++------- src/sys/driver.h | 4 ++-- tools/run-tests.bat | 6 ++++++ tst/winfsp-tests/dirctl-test.c | 6 ++++-- tst/winfsp-tests/mount-test.c | 2 +- tst/winfsp-tests/stream-tests.c | 6 ++++-- tst/winfsp-tests/timeout-test.c | 4 ++-- tst/winfsp-tests/winfsp-tests.c | 33 +++++++++++++++++++++++++++++++-- tst/winfsp-tests/winfsp-tests.h | 2 ++ 9 files changed, 71 insertions(+), 18 deletions(-) diff --git a/src/dll/security.c b/src/dll/security.c index 6fbe5d3e..54eb8747 100644 --- a/src/dll/security.c +++ b/src/dll/security.c @@ -116,18 +116,24 @@ FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, } if (Request->Req.Create.UserMode && - AllowTraverseCheck && !Request->Req.Create.HasTraversePrivilege) + AllowTraverseCheck && !Request->Req.Create.HasTraversePrivilege && + !(L'\\' == FileName[0] && L'\0' == FileName[1])/* no need to traverse check for root */) { - Remain = (PWSTR)FileName; + DEBUGLOG("TraverseCheck: Full=\"%S\"", FileName); + Remain = FileName; for (;;) { - FspPathPrefix(Remain, &Prefix, &Remain, TraverseCheckRoot); - if (L'\0' == Remain[0]) + while (L'\\' != *Remain) { - FspPathCombine(FileName, Remain); - break; + if (L'\0' == *Remain || L':' == *Remain) + goto traverse_check_done; + Remain++; } + *Remain = L'\0'; + Prefix = Remain > FileName ? FileName : TraverseCheckRoot; + DEBUGLOG("TraverseCheck: Part=\"%S\"", Prefix); + FileAttributes = 0; Result = FspGetSecurityByName(FileSystem, Prefix, &FileAttributes, &SecurityDescriptor, &SecurityDescriptorSize); @@ -137,7 +143,11 @@ FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, (FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) FileAttributes = FspPathSuffixIndex(Prefix); - FspPathCombine(FileName, Remain); + *Remain = L'\\'; + do + { + Remain++; + } while (L'\\' == *Remain); if (!NT_SUCCESS(Result) || STATUS_REPARSE == Result) { @@ -187,6 +197,8 @@ FSP_API NTSTATUS FspAccessCheckEx(FSP_FILE_SYSTEM *FileSystem, goto exit; } } + traverse_check_done: + ; } FileAttributes = 0; diff --git a/src/sys/driver.h b/src/sys/driver.h index 24aef312..2c0db058 100644 --- a/src/sys/driver.h +++ b/src/sys/driver.h @@ -33,8 +33,8 @@ /* IoCreateDeviceSecure default SDDL's */ #define FSP_FSCTL_DEVICE_SDDL "D:P(A;;GA;;;SY)(A;;GA;;;BA)(A;;GR;;;WD)" /* System:GENERIC_ALL, Administrators:GENERIC_ALL, World:GENERIC_READ */ -#define FSP_FSVRT_DEVICE_SDDL "D:P(A;;GA;;;SY)(A;;GA;;;BA)(A;;GR;;;WD)" - /* System:GENERIC_ALL, Administrators:GENERIC_ALL, World:GENERIC_READ */ +#define FSP_FSVRT_DEVICE_SDDL "D:P(A;;GA;;;SY)(A;;GA;;;BA)(A;;GRGX;;;WD)" + /* System:GENERIC_ALL, Administrators:GENERIC_ALL, World:GENERIC_READ|GENERIC_EXECUTE */ /* private NTSTATUS codes */ #define FSP_STATUS_PRIVATE_BIT (0x20000000) diff --git a/tools/run-tests.bat b/tools/run-tests.bat index 6923fef0..799a11dd 100644 --- a/tools/run-tests.bat +++ b/tools/run-tests.bat @@ -84,6 +84,9 @@ if errorlevel 1 goto fail echo winfsp-tests-x64 --mountpoint=mymnt winfsp-tests-x64 --mountpoint=mymnt if errorlevel 1 goto fail +echo winfsp-tests-x64 --no-traverse +winfsp-tests-x64 --no-traverse +if errorlevel 1 goto fail exit /b 0 :winfsp-tests-x86 @@ -99,6 +102,9 @@ if errorlevel 1 goto fail echo winfsp-tests-x86 --mountpoint=mymnt winfsp-tests-x86 --mountpoint=mymnt if errorlevel 1 goto fail +echo winfsp-tests-x86 --no-traverse +winfsp-tests-x86 --no-traverse +if errorlevel 1 goto fail exit /b 0 :fsx-memfs-x64 diff --git a/tst/winfsp-tests/dirctl-test.c b/tst/winfsp-tests/dirctl-test.c index 29ef1bb5..fb36a3f9 100644 --- a/tst/winfsp-tests/dirctl-test.c +++ b/tst/winfsp-tests/dirctl-test.c @@ -479,12 +479,14 @@ void dirnotify_test(void) GetCurrentDirectoryW(MAX_PATH - 4, DirBuf + 4); dirnotify_dotest(-1, DirBuf, 0, 0); } - if (WinFspDiskTests) + if (WinFspDiskTests && !OptNoTraverseToken + /* WinFsp does not support change notifications without traverse privilege*/) { dirnotify_dotest(MemfsDisk, 0, 0, 0); dirnotify_dotest(MemfsDisk, 0, 1000, 0); } - if (WinFspNetTests) + if (WinFspNetTests && !OptNoTraverseToken + /* WinFsp does not support change notifications without traverse privilege*/) { dirnotify_dotest(MemfsNet, L"\\\\memfs\\share", 0, 0); dirnotify_dotest(MemfsNet, L"\\\\memfs\\share", 1000, 0); diff --git a/tst/winfsp-tests/mount-test.c b/tst/winfsp-tests/mount-test.c index 24922456..65f3cb96 100644 --- a/tst/winfsp-tests/mount-test.c +++ b/tst/winfsp-tests/mount-test.c @@ -208,7 +208,7 @@ void mount_volume_transact_dotest(PWSTR DeviceName, PWSTR Prefix) ASSERT(0 == Request->Req.Create.Ea.Offset); ASSERT(0 == Request->Req.Create.Ea.Size); ASSERT(Request->Req.Create.UserMode); - ASSERT(Request->Req.Create.HasTraversePrivilege); + ASSERT(!OptNoTraverseToken == Request->Req.Create.HasTraversePrivilege); ASSERT(!Request->Req.Create.OpenTargetDirectory); ASSERT(!Request->Req.Create.CaseSensitive); ASSERT(0 == Request->FileName.Offset); diff --git a/tst/winfsp-tests/stream-tests.c b/tst/winfsp-tests/stream-tests.c index 5ec3988f..748a8922 100644 --- a/tst/winfsp-tests/stream-tests.c +++ b/tst/winfsp-tests/stream-tests.c @@ -2102,12 +2102,14 @@ void stream_dirnotify_test(void) GetCurrentDirectoryW(MAX_PATH - 4, DirBuf + 4); stream_dirnotify_dotest(-1, DirBuf, 0, 0); } - if (WinFspDiskTests) + if (WinFspDiskTests && !OptNoTraverseToken + /* WinFsp does not support change notifications without traverse privilege*/) { stream_dirnotify_dotest(MemfsDisk, 0, 0, 0); stream_dirnotify_dotest(MemfsDisk, 0, 1000, 0); } - if (WinFspNetTests) + if (WinFspNetTests && !OptNoTraverseToken + /* WinFsp does not support change notifications without traverse privilege*/) { stream_dirnotify_dotest(MemfsNet, L"\\\\memfs\\share", 0, 0); stream_dirnotify_dotest(MemfsNet, L"\\\\memfs\\share", 1000, 0); diff --git a/tst/winfsp-tests/timeout-test.c b/tst/winfsp-tests/timeout-test.c index fff2d5e6..8a7fa0cd 100644 --- a/tst/winfsp-tests/timeout-test.c +++ b/tst/winfsp-tests/timeout-test.c @@ -104,7 +104,7 @@ void timeout_pending_dotest(PWSTR DeviceName, PWSTR Prefix) ASSERT(0 == Request->Req.Create.Ea.Offset); ASSERT(0 == Request->Req.Create.Ea.Size); ASSERT(Request->Req.Create.UserMode); - ASSERT(Request->Req.Create.HasTraversePrivilege); + ASSERT(!OptNoTraverseToken == Request->Req.Create.HasTraversePrivilege); ASSERT(!Request->Req.Create.OpenTargetDirectory); ASSERT(!Request->Req.Create.CaseSensitive); ASSERT(0 == Request->FileName.Offset); @@ -214,7 +214,7 @@ void timeout_transact_dotest(PWSTR DeviceName, PWSTR Prefix) ASSERT(0 == Request->Req.Create.Ea.Offset); ASSERT(0 == Request->Req.Create.Ea.Size); ASSERT(Request->Req.Create.UserMode); - ASSERT(Request->Req.Create.HasTraversePrivilege); + ASSERT(!OptNoTraverseToken == Request->Req.Create.HasTraversePrivilege); ASSERT(!Request->Req.Create.OpenTargetDirectory); ASSERT(!Request->Req.Create.CaseSensitive); } diff --git a/tst/winfsp-tests/winfsp-tests.c b/tst/winfsp-tests/winfsp-tests.c index 827a02fe..7cc4ba7f 100644 --- a/tst/winfsp-tests/winfsp-tests.c +++ b/tst/winfsp-tests/winfsp-tests.c @@ -10,6 +10,8 @@ int WinFspNetTests = 1; BOOLEAN OptCaseInsensitive = FALSE; BOOLEAN OptCaseRandomize = FALSE; WCHAR OptMountPointBuf[MAX_PATH], *OptMountPoint; +HANDLE OptNoTraverseToken = 0; + LUID OptNoTraverseLuid; int mywcscmp(PWSTR a, int alen, PWSTR b, int blen) { @@ -63,6 +65,7 @@ HANDLE HookCreateFileW( L"\\\\memfs\\share"; static const TogglePercent = 25; WCHAR FileNameBuf[1024]; + TOKEN_PRIVILEGES Privileges; PWSTR P, EndP; size_t L1, L2; @@ -119,6 +122,15 @@ HANDLE HookCreateFileW( abort(); } + if (OptNoTraverseToken) + { + Privileges.PrivilegeCount = 1; + Privileges.Privileges[0].Attributes = 0; + Privileges.Privileges[0].Luid = OptNoTraverseLuid; + if (!AdjustTokenPrivileges(OptNoTraverseToken, FALSE, &Privileges, 0, 0, 0)) + abort(); + } + HANDLE h = CreateFileW( FileNameBuf, dwDesiredAccess, @@ -127,9 +139,18 @@ HANDLE HookCreateFileW( dwCreationDisposition, dwFlagsAndAttributes, hTemplateFile); + DWORD LastError = GetLastError(); + + if (OptNoTraverseToken) + { + Privileges.PrivilegeCount = 1; + Privileges.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED; + Privileges.Privileges[0].Luid = OptNoTraverseLuid; + if (!AdjustTokenPrivileges(OptNoTraverseToken, FALSE, &Privileges, 0, 0, 0)) + abort(); + } #if 0 - DWORD LastError = GetLastError(); FspDebugLog("CreateFileW(\"%S\", %#lx, %#lx, %p, %#lx, %#lx, %p) = %p[%#lx]\n", FileNameBuf, dwDesiredAccess, @@ -139,9 +160,9 @@ HANDLE HookCreateFileW( dwFlagsAndAttributes, hTemplateFile, h, INVALID_HANDLE_VALUE != h ? 0 : LastError); - SetLastError(LastError); #endif + SetLastError(LastError); return h; } @@ -200,6 +221,14 @@ int main(int argc, char *argv[]) WinFspNetTests = 0; } } + else if (0 == strcmp("--no-traverse", a)) + { + if (LookupPrivilegeValue(0, SE_CHANGE_NOTIFY_NAME, &OptNoTraverseLuid) && + OpenProcessToken(GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, &OptNoTraverseToken)) + { + rmarg(argv, argc, argi); + } + } } } diff --git a/tst/winfsp-tests/winfsp-tests.h b/tst/winfsp-tests/winfsp-tests.h index 87268eca..95ffdf3e 100644 --- a/tst/winfsp-tests/winfsp-tests.h +++ b/tst/winfsp-tests/winfsp-tests.h @@ -24,5 +24,7 @@ extern int WinFspNetTests; extern BOOLEAN OptCaseInsensitive; extern BOOLEAN OptCaseRandomize; extern WCHAR OptMountPointBuf[], *OptMountPoint; +extern HANDLE OptNoTraverseToken; + extern LUID OptNoTraverseLuid; extern int memfs_running;