From 2ffbeb111c75bc471c9ac747ab3b848055e9473d Mon Sep 17 00:00:00 2001 From: DavidXanatos <3890945+DavidXanatos@users.noreply.github.com> Date: Fri, 5 Apr 2024 20:05:37 +0200 Subject: [PATCH] fix for #3650 --- CHANGELOG.md | 3 + Sandboxie/core/dll/file_del.c | 151 ++++++++++++++++++++++++++-------- Sandboxie/core/dll/key_del.c | 34 +++++++- 3 files changed, 148 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d441573..dafcd10f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). - on systems in test signing mode sandboxie will by default try outdated offsets - changed Qt5 version to Qt5.15.13 with latest security patches [#3694](https://github.com/sandboxie-plus/Sandboxie/pull/3694) (thanks LumitoLuma) +### Fixed + - fixed Virtualization scheme Version 2 causing extremely slow file deleting speed [#3650](https://github.com/sandboxie-plus/Sandboxie/issues/3650) + ## [1.13.4 / 5.68.4] - 2024-03-25 diff --git a/Sandboxie/core/dll/file_del.c b/Sandboxie/core/dll/file_del.c index abb0c45a..6b77e140 100644 --- a/Sandboxie/core/dll/file_del.c +++ b/Sandboxie/core/dll/file_del.c @@ -400,6 +400,42 @@ _FX ULONG File_GetPathFlags(const WCHAR* Path, WCHAR** pRelocation) } +//--------------------------------------------------------------------------- +// File_AppendPathEntry_internal +//--------------------------------------------------------------------------- + + +_FX VOID File_AppendPathEntry_internal(HANDLE hPathsFile, const WCHAR* Path, ULONG SetFlags, const WCHAR* Relocation, WCHAR* (*TranslatePath)(const WCHAR*)) +{ + const WCHAR CrLf[] = L"\r\n"; + WCHAR FlagStr[16] = L"|"; + + IO_STATUS_BLOCK IoStatusBlock; + + // write the path + WCHAR* PathEx = TranslatePath ? TranslatePath(Path) : NULL; + NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, PathEx ? PathEx : Path, wcslen(PathEx ? PathEx : Path) * sizeof(WCHAR), NULL, NULL); + if (PathEx) Dll_Free(PathEx); + + // write the flags + _ultow(SetFlags, FlagStr + 1, 16); + NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, FlagStr, wcslen(FlagStr) * sizeof(WCHAR), NULL, NULL); + + // write the relocation + if (Relocation != NULL) { + + NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, FlagStr, sizeof(WCHAR), NULL, NULL); // write | + + WCHAR* RelocationEx = TranslatePath ? TranslatePath(Relocation) : NULL; + NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, RelocationEx ? RelocationEx : Relocation, wcslen(RelocationEx ? RelocationEx : Relocation) * sizeof(WCHAR), NULL, NULL); + if (RelocationEx) Dll_Free(RelocationEx); + } + + // write line ending + NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, (void*)CrLf, sizeof(CrLf) - sizeof(WCHAR), NULL, NULL); +} + + //--------------------------------------------------------------------------- // File_SavePathNode_internal //--------------------------------------------------------------------------- @@ -407,11 +443,6 @@ _FX ULONG File_GetPathFlags(const WCHAR* Path, WCHAR** pRelocation) _FX VOID File_SavePathNode_internal(HANDLE hPathsFile, LIST* parent, WCHAR* Path, ULONG Length, ULONG SetFlags, WCHAR* (*TranslatePath)(const WCHAR *)) { - IO_STATUS_BLOCK IoStatusBlock; - - const WCHAR CrLf[] = L"\r\n"; - WCHAR FlagStr[16] = L"|"; - // append L"\\" Path[Length++] = L'\\'; //Path[Length] = L'0'; WCHAR* PathBase = Path + Length; @@ -431,30 +462,8 @@ _FX VOID File_SavePathNode_internal(HANDLE hPathsFile, LIST* parent, WCHAR* Path if ((child->flags & FILE_RELOCATION_FLAG) != 0) SetFlags = 0; - if ((child->flags & ~SetFlags) != 0 || child->relocation != NULL) { - - // write the path - WCHAR* PathEx = TranslatePath ? TranslatePath(Path) : NULL; - NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, PathEx ? PathEx : Path, wcslen(PathEx ? PathEx : Path) * sizeof(WCHAR), NULL, NULL); - if (PathEx) Dll_Free(PathEx); - - // write the flags - _ultow(child->flags, FlagStr + 1, 16); - NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, FlagStr, wcslen(FlagStr) * sizeof(WCHAR), NULL, NULL); - - // write the relocation - if (child->relocation != NULL) { - - NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, FlagStr, sizeof(WCHAR), NULL, NULL); // write | - - WCHAR* RelocationEx = TranslatePath ? TranslatePath(child->relocation) : NULL; - NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, RelocationEx ? RelocationEx : child->relocation, wcslen(RelocationEx ? RelocationEx : child->relocation) * sizeof(WCHAR), NULL, NULL); - if (RelocationEx) Dll_Free(RelocationEx); - } - - // write line ending - NtWriteFile(hPathsFile, NULL, NULL, NULL, &IoStatusBlock, (void*)CrLf, sizeof(CrLf) - sizeof(WCHAR), NULL, NULL); - } + if ((child->flags & ~SetFlags) != 0 || child->relocation != NULL) + File_AppendPathEntry_internal(hPathsFile, Path, child->flags, child->relocation, TranslatePath); File_SavePathNode_internal(hPathsFile, &child->items, Path, Path_Len, SetFlags | child->flags, TranslatePath); @@ -464,11 +473,35 @@ _FX VOID File_SavePathNode_internal(HANDLE hPathsFile, LIST* parent, WCHAR* Path //--------------------------------------------------------------------------- -// File_SavePathTree_internal +// File_SeekToEndOfFile //--------------------------------------------------------------------------- -_FX VOID File_SavePathTree_internal(LIST* Root, const WCHAR* name, WCHAR* (*TranslatePath)(const WCHAR *)) +_FX BOOLEAN File_SeekToEndOfFile(HANDLE fileHandle) +{ + NTSTATUS status; + IO_STATUS_BLOCK ioStatusBlock; + FILE_STANDARD_INFORMATION fileStandardInfo; + + status = NtQueryInformationFile(fileHandle, &ioStatusBlock, &fileStandardInfo, sizeof(FILE_STANDARD_INFORMATION), FileStandardInformation); + if (NT_SUCCESS(status)) + { + FILE_POSITION_INFORMATION filePosition; + filePosition.CurrentByteOffset = fileStandardInfo.EndOfFile; + + status = NtSetInformationFile(fileHandle, &ioStatusBlock, &filePosition, sizeof(FILE_POSITION_INFORMATION), FilePositionInformation); + } + + return NT_SUCCESS(status); +} + + +//--------------------------------------------------------------------------- +// File_OpenDataFile +//--------------------------------------------------------------------------- + + +_FX BOOLEAN File_OpenDataFile(const WCHAR* name, HANDLE* hPathsFile, BOOLEAN Append) { WCHAR PathsFile[MAX_PATH] = { 0 }; wcscpy(PathsFile, Dll_BoxFilePath); @@ -481,9 +514,28 @@ _FX VOID File_SavePathTree_internal(LIST* Root, const WCHAR* name, WCHAR* (*Tran OBJECT_ATTRIBUTES objattrs; InitializeObjectAttributes(&objattrs, &objname, OBJ_CASE_INSENSITIVE, NULL, NULL); - HANDLE hPathsFile; IO_STATUS_BLOCK IoStatusBlock; - if (!NT_SUCCESS(NtCreateFile(&hPathsFile, GENERIC_WRITE | SYNCHRONIZE, &objattrs, &IoStatusBlock, NULL, 0, FILE_SHARE_READ, FILE_OVERWRITE_IF, FILE_SYNCHRONOUS_IO_NONALERT | FILE_NON_DIRECTORY_FILE, NULL, 0))) + if (!NT_SUCCESS(NtCreateFile(hPathsFile, GENERIC_WRITE | SYNCHRONIZE, &objattrs, &IoStatusBlock, NULL, 0, FILE_SHARE_READ, Append ? FILE_OPEN_IF : FILE_OVERWRITE_IF, FILE_SYNCHRONOUS_IO_NONALERT | FILE_NON_DIRECTORY_FILE, NULL, 0))) + return FALSE; + + if (Append && !File_SeekToEndOfFile(*hPathsFile)) { + NtClose(*hPathsFile); + return FALSE; + } + + return TRUE; +} + + +//--------------------------------------------------------------------------- +// File_SavePathTree_internal +//--------------------------------------------------------------------------- + + +_FX VOID File_SavePathTree_internal(LIST* Root, const WCHAR* name, WCHAR* (*TranslatePath)(const WCHAR *)) +{ + HANDLE hPathsFile; + if (!File_OpenDataFile(name, &hPathsFile, FALSE)) return; WCHAR* Path = (WCHAR *)Dll_Alloc((0x7FFF + 1)*sizeof(WCHAR)); // max nt path @@ -922,7 +974,7 @@ _FX BOOL File_TestBoxRootChange(ULONG WatchBit) //--------------------------------------------------------------------------- -_FX BOOLEAN File_MarkDeleted_internal(LIST* Root, const WCHAR* Path) +_FX BOOLEAN File_MarkDeleted_internal(LIST* Root, const WCHAR* Path, BOOLEAN* pTruncated) { // 1. remove deleted branch @@ -937,6 +989,7 @@ _FX BOOLEAN File_MarkDeleted_internal(LIST* Root, const WCHAR* Path) File_ClearPathBranche_internal(&Node->items); if (Node->relocation) Dll_Free(Node->relocation); Dll_Free(Node); + if (pTruncated) *pTruncated = TRUE; } // 2. set deleted flag @@ -964,11 +1017,37 @@ _FX NTSTATUS File_MarkDeleted_v2(const WCHAR* TruePath) EnterCriticalSection(File_PathRoot_CritSec); - BOOLEAN bSet = File_MarkDeleted_internal(&File_PathRoot, File_NormalizePath(TruePath, NORM_NAME_BUFFER)); + const WCHAR* Path = File_NormalizePath(TruePath, NORM_NAME_BUFFER); + BOOLEAN bTruncated = FALSE; + BOOLEAN bSet = File_MarkDeleted_internal(&File_PathRoot, Path, &bTruncated); LeaveCriticalSection(File_PathRoot_CritSec); - if (bSet) File_SavePathTree(); + if (bSet) + { + // + // Optimization: When marking a lot of host files as deleted, only append single line entries if possible instead of re creating the entire file + // + + ULONG64 PathsFileSize = 0; + ULONG64 PathsFileDate = 0; + if (!bTruncated + && File_GetAttributes_internal(FILE_PATH_FILE_NAME, &PathsFileSize, &PathsFileDate, NULL) + && (File_PathsFileSize == PathsFileSize && File_PathsFileDate == PathsFileDate)) { + + HANDLE hPathsFile; + if (File_OpenDataFile(FILE_PATH_FILE_NAME, &hPathsFile, TRUE)) + { + File_AppendPathEntry_internal(hPathsFile, Path, FILE_DELETED_FLAG, NULL, File_TranslateNtToDosPath2); + + NtClose(hPathsFile); + + File_GetAttributes_internal(FILE_PATH_FILE_NAME, &File_PathsFileSize, &File_PathsFileDate, NULL); + } + } + else + File_SavePathTree(); + } File_ReleaseMutex(hMutex); diff --git a/Sandboxie/core/dll/key_del.c b/Sandboxie/core/dll/key_del.c index 7a2b0394..e8d3a63f 100644 --- a/Sandboxie/core/dll/key_del.c +++ b/Sandboxie/core/dll/key_del.c @@ -80,11 +80,12 @@ static ULONG Key_IsDeletedEx_v2(const WCHAR* TruePath, const WCHAR* ValueName, B // VOID File_ClearPathBranche_internal(LIST* parent); +BOOLEAN File_OpenDataFile(const WCHAR* name, HANDLE* hPathsFile, BOOLEAN Append); +VOID File_AppendPathEntry_internal(HANDLE hPathsFile, const WCHAR* Path, ULONG SetFlags, const WCHAR* Relocation, WCHAR* (*TranslatePath)(const WCHAR*)); VOID File_SavePathTree_internal(LIST* Root, const WCHAR* name, WCHAR* (*TranslatePath)(const WCHAR *)); BOOLEAN File_LoadPathTree_internal(LIST* Root, const WCHAR* name, WCHAR* (*TranslatePath)(const WCHAR *)); -VOID File_SetPathFlags_internal(LIST* Root, const WCHAR* Path, ULONG setFlags, ULONG clrFlags, const WCHAR* Relocation); ULONG File_GetPathFlags_internal(LIST* Root, const WCHAR* Path, WCHAR** pRelocation, BOOLEAN CheckChildren); -BOOLEAN File_MarkDeleted_internal(LIST* Root, const WCHAR* Path); +BOOLEAN File_MarkDeleted_internal(LIST* Root, const WCHAR* Path, BOOLEAN* pTruncated); VOID File_SetRelocation_internal(LIST* Root, const WCHAR* OldTruePath, const WCHAR* NewTruePath); BOOL File_InitBoxRootWatcher(); @@ -241,11 +242,36 @@ _FX NTSTATUS Key_MarkDeletedEx_v2(const WCHAR* TruePath, const WCHAR* ValueName) EnterCriticalSection(Key_PathRoot_CritSec); - BOOLEAN bSet = File_MarkDeleted_internal(&Key_PathRoot, FullPath); + BOOLEAN bTruncated = FALSE; + BOOLEAN bSet = File_MarkDeleted_internal(&Key_PathRoot, FullPath, &bTruncated); LeaveCriticalSection(Key_PathRoot_CritSec); - if (bSet) Key_SavePathTree(); + if (bSet) + { + // + // Optimization: When marking a lot of host keys as deleted, only append single line entries if possible instead of re creating the entire file + // + + ULONG64 PathsFileSize = 0; + ULONG64 PathsFileDate = 0; + if (!bTruncated + && File_GetAttributes_internal(KEY_PATH_FILE_NAME, &PathsFileSize, &PathsFileDate, NULL) + && (Key_PathsFileSize == PathsFileSize && Key_PathsFileDate == PathsFileDate)) { + + HANDLE hPathsFile; + if (File_OpenDataFile(KEY_PATH_FILE_NAME, &hPathsFile, TRUE)) + { + File_AppendPathEntry_internal(hPathsFile, FullPath, KEY_DELETED_FLAG, NULL, NULL); + + NtClose(hPathsFile); + + File_GetAttributes_internal(KEY_PATH_FILE_NAME, &Key_PathsFileSize, &Key_PathsFileDate, NULL); + } + } + else + Key_SavePathTree(); + } File_ReleaseMutex(hMutex);