From d6a90e54267cca2fe9f0a11b8a948fd8b96b51fd Mon Sep 17 00:00:00 2001 From: Akash Mozumdar Date: Thu, 19 Jul 2018 20:51:49 -0400 Subject: [PATCH 1/2] remove direct access to process records --- gui/ProcessWindow.cpp | 2 +- gui/utility.cpp | 14 ++++++-------- gui/window.cpp | 9 +++------ vnr/texthook/hookman.cc | 40 +++++++++++++++++----------------------- vnr/texthook/hookman.h | 3 +-- 5 files changed, 28 insertions(+), 40 deletions(-) diff --git a/gui/ProcessWindow.cpp b/gui/ProcessWindow.cpp index 306cf55..91eccee 100644 --- a/gui/ProcessWindow.cpp +++ b/gui/ProcessWindow.cpp @@ -118,7 +118,7 @@ void ProcessWindow::RefreshThread(int index) item.iItem = index; ListView_GetItem(hlProcess, &item); DWORD pid = item.lParam; - bool isAttached = man->GetProcessRecord(pid) != NULL; + bool isAttached = man->GetHostPipe(pid) != NULL; RefreshThreadWithPID(pid, isAttached); } diff --git a/gui/utility.cpp b/gui/utility.cpp index 2017091..2cc43b3 100644 --- a/gui/utility.cpp +++ b/gui/utility.cpp @@ -171,12 +171,11 @@ std::wstring GetCode(const HookParam& hp, DWORD pid) std::wstring GetModuleFileNameAsString(DWORD pid, PVOID allocationBase) { - const ProcessRecord* pr = man->GetProcessRecord(pid); - if (pr) + UniqueHandle hProc(OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid)); + if (hProc.get()) { - HANDLE hProc = pr->process_handle; WCHAR path[MAX_PATH]; - if (GetModuleFileNameEx(hProc, (HMODULE)allocationBase, path, MAX_PATH)) + if (GetModuleFileNameEx(hProc.get(), (HMODULE)allocationBase, path, MAX_PATH)) return path; } return L""; @@ -184,12 +183,11 @@ std::wstring GetModuleFileNameAsString(DWORD pid, PVOID allocationBase) PVOID GetAllocationBase(DWORD pid, LPCVOID addr) { - const ProcessRecord *pr = man->GetProcessRecord(pid); - if (pr) + UniqueHandle hProc(OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid)); + if (hProc.get()) { MEMORY_BASIC_INFORMATION info; - HANDLE hProc = pr->process_handle; - if (VirtualQueryEx(hProc, addr, &info, sizeof(info))) + if (VirtualQueryEx(hProc.get(), addr, &info, sizeof(info))) { if (info.Type & MEM_IMAGE) return info.AllocationBase; diff --git a/gui/window.cpp b/gui/window.cpp index f323f5f..ba0c222 100644 --- a/gui/window.cpp +++ b/gui/window.cpp @@ -438,15 +438,12 @@ void ThreadCreate(TextThread* thread) //thread->RegisterFilterCallBack(ThreadFilter, 0); AddToCombo(*thread, false); auto tp = thread->GetThreadParameter(); - auto pr = man->GetProcessRecord(tp.pid); - if (pr == NULL) - return; - if (IsUnicodeHook(*pr, tp.hook)) - thread->Status() |= USING_UNICODE; + auto hook = man->GetHookParam(tp.pid, tp.hook); + if (hook.type & USING_UNICODE) thread->Status() |= USING_UNICODE; auto pf = pfman->GetProfile(tp.pid); if (!pf) return; - const std::wstring& hook_name = GetHookNameByAddress(*pr, thread->GetThreadParameter().hook); + const std::wstring& hook_name = man->GetHookName(tp.pid, tp.hook); auto thread_profile = pf->FindThread(&thread->GetThreadParameter(), hook_name); if (thread_profile != pf->Threads().end()) { diff --git a/vnr/texthook/hookman.cc b/vnr/texthook/hookman.cc index ad9500a..91ed2f8 100644 --- a/vnr/texthook/hookman.cc +++ b/vnr/texthook/hookman.cc @@ -101,19 +101,19 @@ void HookManager::RemoveProcessContext(DWORD pid) void HookManager::RegisterProcess(DWORD pid, HANDLE hostPipe) { HM_LOCK; - ProcessRecord* record = processRecordsByIds[pid] = new ProcessRecord; - record->hostPipe = hostPipe; - record->hookman_section = OpenFileMappingW(FILE_MAP_READ, FALSE, (ITH_SECTION_ + std::to_wstring(pid)).c_str()); - record->hookman_map = MapViewOfFile(record->hookman_section, FILE_MAP_READ, 0, 0, HOOK_SECTION_SIZE / 2); // jichi 1/16/2015: Changed to half to hook section size - record->process_handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid); - record->hookman_mutex = OpenMutexW(MUTEX_ALL_ACCESS, FALSE, (ITH_HOOKMAN_MUTEX_ + std::to_wstring(pid)).c_str()); + ProcessRecord record = processRecordsByIds[pid]; + record.hostPipe = hostPipe; + record.hookman_section = OpenFileMappingW(FILE_MAP_READ, FALSE, (ITH_SECTION_ + std::to_wstring(pid)).c_str()); + record.hookman_map = MapViewOfFile(record.hookman_section, FILE_MAP_READ, 0, 0, HOOK_SECTION_SIZE / 2); // jichi 1/16/2015: Changed to half to hook section size + record.process_handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid); + record.hookman_mutex = OpenMutexW(MUTEX_ALL_ACCESS, FALSE, (ITH_HOOKMAN_MUTEX_ + std::to_wstring(pid)).c_str()); if (attach) attach(pid); } void HookManager::UnRegisterProcess(DWORD pid) { HM_LOCK; - ProcessRecord pr = *processRecordsByIds[pid]; + ProcessRecord pr = processRecordsByIds[pid]; CloseHandle(pr.hookman_mutex); UnmapViewOfFile(pr.hookman_map); CloseHandle(pr.process_handle); @@ -154,26 +154,20 @@ void HookManager::ClearCurrent() if (reset) reset(current); } -ProcessRecord *HookManager::GetProcessRecord(DWORD pid) -{ - HM_LOCK; - return processRecordsByIds[pid]; -} - HANDLE HookManager::GetHostPipe(DWORD pid) { HM_LOCK; - return processRecordsByIds[pid] ? processRecordsByIds[pid]->hostPipe : nullptr; + return processRecordsByIds[pid].hostPipe; } HookParam HookManager::GetHookParam(DWORD pid, DWORD addr) { HM_LOCK; HookParam ret = {}; - ProcessRecord* pr = GetProcessRecord(pid); - if (pr == nullptr) return ret; - MutexLocker locker(pr->hookman_mutex); - const Hook* hooks = (const Hook*)pr->hookman_map; + ProcessRecord pr = processRecordsByIds[pid]; + if (pr.hookman_map == nullptr) return ret; + MutexLocker locker(pr.hookman_mutex); + const Hook* hooks = (const Hook*)pr.hookman_map; for (int i = 0; i < MAX_HOOK; ++i) if (hooks[i].Address() == addr) ret = hooks[i].hp; @@ -184,17 +178,17 @@ std::wstring HookManager::GetHookName(DWORD pid, DWORD addr) { HM_LOCK; std::string buffer; - ProcessRecord* pr = GetProcessRecord(pid); - if (pr == nullptr) return L""; - MutexLocker locker(pr->hookman_mutex); + ProcessRecord pr = processRecordsByIds[pid]; + if (pr.hookman_map == nullptr) return L""; + MutexLocker locker(pr.hookman_mutex); USES_CONVERSION; - const Hook* hooks = (const Hook*)pr->hookman_map; + const Hook* hooks = (const Hook*)pr.hookman_map; for (int i = 0; i < MAX_HOOK; ++i) { if (hooks[i].Address() == addr) { buffer.resize(hooks[i].NameLength()); - ReadProcessMemory(pr->process_handle, hooks[i].Name(), &buffer[0], hooks[i].NameLength(), nullptr); + ReadProcessMemory(pr.process_handle, hooks[i].Name(), &buffer[0], hooks[i].NameLength(), nullptr); } } return std::wstring(A2W(buffer.c_str())); diff --git a/vnr/texthook/hookman.h b/vnr/texthook/hookman.h index d6929b9..bdbdf28 100644 --- a/vnr/texthook/hookman.h +++ b/vnr/texthook/hookman.h @@ -38,7 +38,6 @@ public: ~HookManager(); TextThread *FindSingle(DWORD number); - ProcessRecord *GetProcessRecord(DWORD pid); HANDLE GetHostPipe(DWORD pid); void ClearCurrent(); void SelectCurrent(DWORD num); @@ -62,7 +61,7 @@ public: private: std::unordered_map textThreadsByParams; - std::unordered_map processRecordsByIds; + std::unordered_map processRecordsByIds; CRITICAL_SECTION hmCs; From 5ea77e23a5a81384cfb1e2391a509c28af19a88c Mon Sep 17 00:00:00 2001 From: Akash Mozumdar Date: Thu, 19 Jul 2018 21:44:03 -0400 Subject: [PATCH 2/2] bugfix --- CMakeLists.txt | 2 +- vnr/texthook/hookman.cc | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c397e9e..3e045ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,7 +32,7 @@ add_compile_options( /MP /GS- $<$:/MT> - $<$:/MTd> + #$<$:/MTd> ) add_definitions( diff --git a/vnr/texthook/hookman.cc b/vnr/texthook/hookman.cc index 91ed2f8..afa8fcd 100644 --- a/vnr/texthook/hookman.cc +++ b/vnr/texthook/hookman.cc @@ -101,12 +101,13 @@ void HookManager::RemoveProcessContext(DWORD pid) void HookManager::RegisterProcess(DWORD pid, HANDLE hostPipe) { HM_LOCK; - ProcessRecord record = processRecordsByIds[pid]; + ProcessRecord record; record.hostPipe = hostPipe; record.hookman_section = OpenFileMappingW(FILE_MAP_READ, FALSE, (ITH_SECTION_ + std::to_wstring(pid)).c_str()); record.hookman_map = MapViewOfFile(record.hookman_section, FILE_MAP_READ, 0, 0, HOOK_SECTION_SIZE / 2); // jichi 1/16/2015: Changed to half to hook section size record.process_handle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid); record.hookman_mutex = OpenMutexW(MUTEX_ALL_ACCESS, FALSE, (ITH_HOOKMAN_MUTEX_ + std::to_wstring(pid)).c_str()); + processRecordsByIds[pid] = record; if (attach) attach(pid); }