From c9a7a606cb5285e14576d3ec64719aa212b9d1bb Mon Sep 17 00:00:00 2001 From: Akash Mozumdar Date: Sat, 16 Feb 2019 22:51:10 -0500 Subject: [PATCH] fix bugs with admin rights and pipe connection, plus some refactors --- GUI/host/host.cpp | 17 +++++++---------- GUI/mainwindow.cpp | 4 +--- include/const.h | 24 ++++-------------------- include/defs.h | 15 ++++++++++++++- include/types.h | 21 +++++++++++---------- vnrhook/main.cc | 42 ++++++++++++++++++------------------------ vnrhook/texthook.cc | 6 +++--- 7 files changed, 58 insertions(+), 71 deletions(-) diff --git a/GUI/host/host.cpp b/GUI/host/host.cpp index 44dad6b..a7b7798 100644 --- a/GUI/host/host.cpp +++ b/GUI/host/host.cpp @@ -33,8 +33,9 @@ namespace } template - std::enable_if_t Send(T data) + void Send(T data) { + static_assert(sizeof(data) < PIPE_BUFFER_SIZE); std::thread([=] { DWORD DUMMY; @@ -75,15 +76,12 @@ namespace { std::thread([] { - SECURITY_DESCRIPTOR pipeSD = {}; - InitializeSecurityDescriptor(&pipeSD, SECURITY_DESCRIPTOR_REVISION); - SetSecurityDescriptorDacl(&pipeSD, TRUE, NULL, FALSE); // Allow non-admin processes to connect to pipe created by admin host - SECURITY_ATTRIBUTES pipeSA = { sizeof(pipeSA), &pipeSD, FALSE }; - struct PipeCloser { void operator()(HANDLE h) { DisconnectNamedPipe(h); CloseHandle(h); } }; AutoHandle - hookPipe = CreateNamedPipeW(HOOK_PIPE, PIPE_ACCESS_INBOUND, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, PIPE_UNLIMITED_INSTANCES, 0, PIPE_BUFFER_SIZE, MAXDWORD, &pipeSA), - hostPipe = CreateNamedPipeW(HOST_PIPE, PIPE_ACCESS_OUTBOUND, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, PIPE_UNLIMITED_INSTANCES, PIPE_BUFFER_SIZE, 0, MAXDWORD, &pipeSA); + hookPipe = CreateNamedPipeW(HOOK_PIPE, PIPE_ACCESS_INBOUND, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, PIPE_UNLIMITED_INSTANCES, 0, PIPE_BUFFER_SIZE, MAXDWORD, &allAccess), + hostPipe = CreateNamedPipeW(HOST_PIPE, PIPE_ACCESS_OUTBOUND, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, PIPE_UNLIMITED_INSTANCES, PIPE_BUFFER_SIZE, 0, MAXDWORD, &allAccess); + static AutoHandle<> pipeAvailableEvent = CreateEventW(&allAccess, FALSE, FALSE, PIPE_AVAILABLE_EVENT); + SetEvent(pipeAvailableEvent); ConnectNamedPipe(hookPipe, nullptr); BYTE buffer[PIPE_BUFFER_SIZE] = {}; @@ -171,8 +169,6 @@ namespace Host WinMutex(ITH_HOOKMAN_MUTEX_ + std::to_wstring(processId)); if (GetLastError() == ERROR_ALREADY_EXISTS) return AddConsoleOutput(ALREADY_INJECTED); - static std::wstring location = Util::GetModuleFilename(LoadLibraryExW(ITH_DLL, nullptr, DONT_RESOLVE_DLL_REFERENCES)).value(); - if (AutoHandle<> process = OpenProcess(PROCESS_ALL_ACCESS, FALSE, processId)) { #ifdef _WIN64 @@ -180,6 +176,7 @@ namespace Host IsWow64Process(process, &invalidProcess); if (invalidProcess) return AddConsoleOutput(ARCHITECTURE_MISMATCH); #endif + static std::wstring location = Util::GetModuleFilename(LoadLibraryExW(ITH_DLL, nullptr, DONT_RESOLVE_DLL_REFERENCES)).value(); if (LPVOID remoteData = VirtualAllocEx(process, nullptr, (location.size() + 1) * sizeof(wchar_t), MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE)) { WriteProcessMemory(process, remoteData, location.c_str(), (location.size() + 1) * sizeof(wchar_t), nullptr); diff --git a/GUI/mainwindow.cpp b/GUI/mainwindow.cpp index 615f7b6..2889dbb 100644 --- a/GUI/mainwindow.cpp +++ b/GUI/mainwindow.cpp @@ -78,9 +78,7 @@ MainWindow::MainWindow(QWidget *parent) : MainWindow::~MainWindow() { - QSettings settings(CONFIG_FILE, QSettings::IniFormat); - settings.setValue(WINDOW, geometry()); - settings.sync(); + QSettings(CONFIG_FILE, QSettings::IniFormat).setValue(WINDOW, geometry()); SetErrorMode(SEM_NOGPFAULTERRORBOX); ExitProcess(0); } diff --git a/include/const.h b/include/const.h index 9828a3f..5473cbb 100644 --- a/include/const.h +++ b/include/const.h @@ -4,25 +4,11 @@ // 8/23/2013 jichi // Branch: ITH/common.h, rev 128 -enum { MESSAGE_SIZE = 500, PIPE_BUFFER_SIZE = 2000, SHIFT_JIS = 932, MAX_MODULE_SIZE = 120, HOOK_NAME_SIZE = 30 }; +enum Misc { MESSAGE_SIZE = 500, PIPE_BUFFER_SIZE = 2000, SHIFT_JIS = 932, MAX_MODULE_SIZE = 120, HOOK_NAME_SIZE = 30, FIXED_SPLIT_VALUE = 0x10001 }; -enum HostCommandType -{ - HOST_COMMAND = -1, // null type - HOST_COMMAND_NEW_HOOK = 0, - HOST_COMMAND_REMOVE_HOOK = 1, - HOST_COMMAND_MODIFY_HOOK = 2, - HOST_COMMAND_HIJACK_PROCESS = 3, - HOST_COMMAND_DETACH = 4 -}; +enum HostCommandType { HOST_COMMAND_NEW_HOOK, HOST_COMMAND_REMOVE_HOOK, HOST_COMMAND_MODIFY_HOOK, HOST_COMMAND_HIJACK_PROCESS, HOST_COMMAND_DETACH }; -enum HostNotificationType -{ - HOST_NOTIFICATION = -1, // null type - HOST_NOTIFICATION_TEXT = 0, - HOST_NOTIFICATION_NEWHOOK = 1, - HOST_NOTIFICATION_RMVHOOK = 2 -}; +enum HostNotificationType { HOST_NOTIFICATION_TEXT, HOST_NOTIFICATION_NEWHOOK, HOST_NOTIFICATION_RMVHOOK }; enum HookParamType : unsigned { @@ -30,7 +16,7 @@ enum HookParamType : unsigned USING_UNICODE = 0x2, // type(data) is wchar_t or wchar_t* BIG_ENDIAN = 0x4, // type(data) is char DATA_INDIRECT = 0x8, - USING_SPLIT = 0x10, // aware of split time? + USING_SPLIT = 0x10, // use ctx2 or not SPLIT_INDIRECT = 0x20, MODULE_OFFSET = 0x40, // address is relative to module FUNCTION_OFFSET = 0x80, // address is relative to function @@ -43,5 +29,3 @@ enum HookParamType : unsigned HOOK_ENGINE = 0x4000, HOOK_ADDITIONAL = 0x8000 }; - -enum { FIXED_SPLIT_VALUE = 0x10001 }; // 6/1/2014: Fixed split value for hok parameter. Fuse all threads, and prevent floating diff --git a/include/defs.h b/include/defs.h index ddf719c..90eface 100644 --- a/include/defs.h +++ b/include/defs.h @@ -14,9 +14,14 @@ constexpr auto HOST_PIPE = L"\\\\.\\pipe\\TEXTRACTOR_HOST"; constexpr auto ITH_SECTION_ = L"VNR_SECTION_"; // _%d -// Mutex +// Mutexes constexpr auto ITH_HOOKMAN_MUTEX_ = L"VNR_HOOKMAN_"; // ITH_HOOKMAN_%d +constexpr auto CONNECTING_MUTEX = L"TEXTRACTOR_CONNECTING_PIPES"; + +// Events + +constexpr auto PIPE_AVAILABLE_EVENT = L"TEXTRACTOR_PIPE_AVAILABLE"; // Files @@ -31,6 +36,14 @@ constexpr auto REPLACE_SAVE_FILE = u8"SavedReplacements.txt"; constexpr auto DEFAULT_EXTENSIONS = u8"Remove Repetition>Lua>Copy to Clipboard>Bing Translate>Extra Window>Extra Newlines"; +inline SECURITY_ATTRIBUTES allAccess = std::invoke([] // allows non-admin processes to access kernel objects made by admin processes +{ + static SECURITY_DESCRIPTOR sd = {}; + InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); + SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE); + return SECURITY_ATTRIBUTES{ sizeof(SECURITY_ATTRIBUTES), &sd, FALSE }; +}); + // Functions template diff --git a/include/types.h b/include/types.h index 01da72c..15819bc 100644 --- a/include/types.h +++ b/include/types.h @@ -48,6 +48,17 @@ private: std::unique_ptr h; }; +class WinMutex // Like CMutex but works with scoped_lock +{ +public: + WinMutex(std::wstring name = L"", LPSECURITY_ATTRIBUTES sa = nullptr) : m(CreateMutexW(sa, FALSE, name.empty() ? NULL : name.c_str())) {} + void lock() { if (m) WaitForSingleObject(m, INFINITE); } + void unlock() { if (m) ReleaseMutex(m); } + +private: + AutoHandle<> m; +}; + // jichi 3/7/2014: Add guessed comment struct HookParam { @@ -83,16 +94,6 @@ struct ThreadParam uint64_t ctx2; // The subcontext of the hook: 0 by default, generated in a method specific to the hook }; -class WinMutex // Like CMutex but works with scoped_lock -{ -public: - WinMutex(std::wstring name) : m(CreateMutexW(nullptr, FALSE, name.c_str())) {} - void lock() { if (m) WaitForSingleObject(m, INFINITE); } - void unlock() { if (m) ReleaseMutex(m); } - -private: - AutoHandle<> m; -}; struct InsertHookCmd // From host { diff --git a/vnrhook/main.cc b/vnrhook/main.cc index 4d4a530..e2aa384 100644 --- a/vnrhook/main.cc +++ b/vnrhook/main.cc @@ -10,12 +10,12 @@ #include "texthook.h" #include "util.h" -std::unique_ptr viewMutex; +WinMutex viewMutex; namespace { AutoHandle<> hookPipe = INVALID_HANDLE_VALUE, mappedFile = INVALID_HANDLE_VALUE; - TextHook* hooks; + TextHook (*hooks)[MAX_HOOK]; bool running; int currentHook = 0; DWORD DUMMY; @@ -30,21 +30,16 @@ DWORD WINAPI Pipe(LPVOID) AutoHandle<> hostPipe = INVALID_HANDLE_VALUE; hookPipe = INVALID_HANDLE_VALUE; - while (!hookPipe || !hostPipe) + while (!hostPipe || !hookPipe) { - if (!hookPipe) - { - hookPipe = CreateFileW(HOOK_PIPE, GENERIC_WRITE, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); - } - if (hookPipe && !hostPipe) - { - hostPipe = CreateFileW(HOST_PIPE, GENERIC_READ | FILE_WRITE_ATTRIBUTES, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); - DWORD mode = PIPE_READMODE_MESSAGE; - SetNamedPipeHandleState(hostPipe, &mode, NULL, NULL); - continue; - } - Sleep(50); + WinMutex connectionMutex(CONNECTING_MUTEX, &allAccess); + std::scoped_lock lock(connectionMutex); + WaitForSingleObject(AutoHandle<>(CreateEventW(&allAccess, FALSE, FALSE, PIPE_AVAILABLE_EVENT)), INFINITE); + hostPipe = CreateFileW(HOST_PIPE, GENERIC_READ | FILE_WRITE_ATTRIBUTES, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + hookPipe = CreateFileW(HOOK_PIPE, GENERIC_WRITE, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); } + DWORD mode = PIPE_READMODE_MESSAGE; + SetNamedPipeHandleState(hostPipe, &mode, NULL, NULL); *(DWORD*)buffer = GetCurrentProcessId(); WriteFile(hookPipe, buffer, sizeof(DWORD), &count, nullptr); @@ -69,7 +64,7 @@ DWORD WINAPI Pipe(LPVOID) } } hookPipe = INVALID_HANDLE_VALUE; - for (int i = 0; i < MAX_HOOK; ++i) if (hooks[i].address) hooks[i].Clear(); + for (auto& hook : *hooks) if (hook.address) hook.Clear(); FreeLibraryAndExitThread(GetModuleHandleW(ITH_DLL), 0); return 0; } @@ -105,19 +100,19 @@ BOOL WINAPI DllMain(HINSTANCE hModule, DWORD fdwReason, LPVOID) { case DLL_PROCESS_ATTACH: { - viewMutex = std::make_unique(ITH_HOOKMAN_MUTEX_ + std::to_wstring(GetCurrentProcessId())); + viewMutex = WinMutex(ITH_HOOKMAN_MUTEX_ + std::to_wstring(GetCurrentProcessId()), &allAccess); if (GetLastError() == ERROR_ALREADY_EXISTS) return FALSE; DisableThreadLibraryCalls(hModule); // jichi 9/25/2013: Interprocedural communication with vnrsrv. - mappedFile = CreateFileMappingW(INVALID_HANDLE_VALUE, nullptr, PAGE_EXECUTE_READWRITE, 0, HOOK_SECTION_SIZE, (ITH_SECTION_ + std::to_wstring(GetCurrentProcessId())).c_str()); - hooks = (TextHook*)MapViewOfFile(mappedFile, FILE_MAP_ALL_ACCESS | FILE_MAP_EXECUTE, 0, 0, HOOK_BUFFER_SIZE); + mappedFile = CreateFileMappingW(INVALID_HANDLE_VALUE, &allAccess, PAGE_EXECUTE_READWRITE, 0, HOOK_SECTION_SIZE, (ITH_SECTION_ + std::to_wstring(GetCurrentProcessId())).c_str()); + hooks = (TextHook(*)[MAX_HOOK])MapViewOfFile(mappedFile, FILE_MAP_ALL_ACCESS | FILE_MAP_EXECUTE, 0, 0, HOOK_BUFFER_SIZE); memset(hooks, 0, HOOK_BUFFER_SIZE); MH_Initialize(); running = true; - CreateThread(nullptr, 0, Pipe, nullptr, 0, nullptr); // Using std::thread here = deadlock + CloseHandle(CreateThread(nullptr, 0, Pipe, nullptr, 0, nullptr)); // Using std::thread here = deadlock } break; case DLL_PROCESS_DETACH: @@ -143,7 +138,7 @@ void NewHook(HookParam hp, LPCSTR lpname, DWORD flag) if (strlen(utf8Text) < 8 || strlen(codepageText) < 8 || wcslen(hp.text) < 4) return ConsoleOutput(NOT_ENOUGH_TEXT); for (auto[addrs, type] : Array, HookParamType>>{ { Util::SearchMemory(utf8Text, strlen(utf8Text), PAGE_READWRITE), USING_UTF8 }, - { Util::SearchMemory(codepageText, strlen(codepageText), PAGE_READWRITE), (HookParamType)0 }, + { Util::SearchMemory(codepageText, strlen(codepageText), PAGE_READWRITE), USING_STRING }, { Util::SearchMemory(hp.text, wcslen(hp.text) * 2, PAGE_READWRITE), USING_UNICODE } }) for (auto addr : addrs) @@ -164,14 +159,13 @@ void NewHook(HookParam hp, LPCSTR lpname, DWORD flag) if (lpname && *lpname) strncpy_s(hp.name, lpname, HOOK_NAME_SIZE - 1); ConsoleOutput(INSERTING_HOOK, hp.name); if (hp.address) RemoveHook(hp.address, 0); - if (!hooks[currentHook].Insert(hp, flag)) ConsoleOutput(HOOK_FAILED); + if (!(*hooks)[currentHook].Insert(hp, flag)) ConsoleOutput(HOOK_FAILED); } } void RemoveHook(uint64_t addr, int maxOffset) { - for (int i = 0; i < MAX_HOOK; i++) - if (abs((long long)(hooks[i].address - addr)) <= maxOffset) return hooks[i].Clear(); + for (auto& hook : *hooks) if (abs((long long)(hook.address - addr)) <= maxOffset) return hook.Clear(); } // EOF diff --git a/vnrhook/texthook.cc b/vnrhook/texthook.cc index 9ea6f95..6f6a487 100644 --- a/vnrhook/texthook.cc +++ b/vnrhook/texthook.cc @@ -11,7 +11,7 @@ #include "text.h" #include "ithsys/ithsys.h" -extern std::unique_ptr viewMutex; +extern WinMutex viewMutex; // - Unnamed helpers - @@ -101,7 +101,7 @@ void SetTrigger() bool TextHook::Insert(HookParam h, DWORD set_flag) { - std::scoped_lock lock(*viewMutex); + std::scoped_lock lock(viewMutex); hp = h; address = hp.address; hp.type |= set_flag; @@ -283,7 +283,7 @@ void TextHook::RemoveReadCode() void TextHook::Clear() { - std::scoped_lock lock(*viewMutex); + std::scoped_lock lock(viewMutex); if (*hp.name) ConsoleOutput(REMOVING_HOOK, hp.name); if (hp.type & DIRECT_READ) RemoveReadCode(); else RemoveHookCode();