Skip to content

Commit 80af96d

Browse files
author
Rafael Stahl
committed
vehhook: fix lost hooks, fix thread safety; vthook: avoid accessing invalid memory in vt backup
1 parent 8eda808 commit 80af96d

6 files changed

Lines changed: 148 additions & 59 deletions

File tree

‎.clang-format‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ AllowShortIfStatementsOnASingleLine: false
66
AlwaysBreakTemplateDeclarations: true
77
BreakBeforeBraces: Allman
88
BreakConstructorInitializersBeforeComma: true
9+
BreakConstructorInitializers: BeforeComma
910
ColumnLimit: 120
1011
ConstructorInitializerAllOnOneLineOrOnePerLine: true
1112
Cpp11BracedListStyle: false
@@ -14,6 +15,7 @@ IndentCaseLabels: false
1415
IndentWidth: 4
1516
KeepEmptyLinesAtTheStartOfBlocks: false
1617
MaxEmptyLinesToKeep: 2
18+
PackConstructorInitializers: CurrentLine
1719
PenaltyReturnTypeOnItsOwnLine: 10000
1820
PointerBindsToType: true
1921
SortIncludes: false

‎CMakeLists.txt‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,20 @@
1-
CMAKE_MINIMUM_REQUIRED(VERSION 3.1 FATAL_ERROR)
2-
1+
CMAKE_MINIMUM_REQUIRED(VERSION 3.10)
32
PROJECT(hacklib_project)
43

54
# set the default configuration to debug
65
IF(NOT CMAKE_BUILD_TYPE)
76
SET(CMAKE_BUILD_TYPE Debug)
87
ENDIF()
98

10-
SET(PROJ_ROOT "${CMAKE_CURRENT_SOURCE_DIR}")
9+
set(PROJ_ROOT "${CMAKE_CURRENT_SOURCE_DIR}")
1110
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
1211
SET(ARCH_64BIT TRUE)
1312
ADD_DEFINITIONS(-DARCH_64BIT)
14-
SET(PROJ_BIN ${PROJ_ROOT}/bin64)
15-
SET(PROJ_LIB ${PROJ_ROOT}/lib64)
13+
SET(PROJ_BIN ${PROJ_ROOT}/bin64)
14+
SET(PROJ_LIB ${PROJ_ROOT}/lib64)
1615
ELSE()
17-
SET(PROJ_BIN ${PROJ_ROOT}/bin)
18-
SET(PROJ_LIB ${PROJ_ROOT}/lib)
16+
SET(PROJ_BIN ${PROJ_ROOT}/bin)
17+
SET(PROJ_LIB ${PROJ_ROOT}/lib)
1918
ENDIF()
2019

2120
# default output directories
@@ -38,7 +37,7 @@ ENDIF()
3837
SET_PROPERTY(GLOBAL PROPERTY USE_FOLDERS ON)
3938
SET(CMAKE_POSITION_INDEPENDENT_CODE ON)
4039
SET(CMAKE_DEBUG_POSTFIX "d")
41-
SET(CMAKE_CXX_STANDARD 14)
40+
SET(CMAKE_CXX_STANDARD 23)
4241
SET(CMAKE_CXX_STANDARD_REQUIRED ON)
4342

4443
IF(WIN32)

‎src/hacklib/src/Hooker.cpp‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ class VTHookManager
5050
}
5151
else
5252
{
53+
// Limit backup size by available readable memory region.
54+
uintptr_t vtAddr = (uintptr_t)*instance;
55+
const auto& memRegion = hl::GetMemoryByAddress(vtAddr);
56+
uintptr_t maxSize = memRegion.base + memRegion.size - vtAddr;
57+
vtBackupSize = std::min(vtBackupSize, (int)(maxSize / sizeof(void*)));
58+
5359
// Create new fake VT (mirroring the original one).
5460
fakeVT = std::make_unique<FakeVT>(instance, vtBackupSize);
5561

@@ -99,7 +105,8 @@ class VTHook : public IHook
99105
{
100106
public:
101107
VTHook(uintptr_t classInstance, int functionIndex, uintptr_t cbHook, int vtBackupSize)
102-
: instance((uintptr_t**)classInstance), functionIndex(functionIndex)
108+
: instance((uintptr_t**)classInstance)
109+
, functionIndex(functionIndex)
103110
{
104111
g_vtHookManager.addHook(instance, functionIndex, cbHook, vtBackupSize);
105112
}
@@ -136,7 +143,10 @@ class DetourHook : public IHook
136143
{
137144
public:
138145
DetourHook(uintptr_t location, int offset, Hooker::HookCallback_t cbHook)
139-
: location(location), offset(offset), cbHook(cbHook), wrapperCode(0x1000, 0xcc)
146+
: location(location)
147+
, offset(offset)
148+
, cbHook(cbHook)
149+
, wrapperCode(0x1000, 0xcc)
140150
{
141151
}
142152
~DetourHook() override

‎src/hacklib/src/Hooker_VEH.cpp‎

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,10 @@ class VEHHookManager
3939

4040
return nullptr;
4141
}
42-
Page* getPage(uintptr_t adr)
42+
bool isPageHooked(uintptr_t adr)
4343
{
44-
std::lock_guard<std::mutex> lock(m_pagesMutex);
45-
46-
auto it = m_pages.upper_bound(adr);
47-
if (it != m_pages.end())
48-
return (--it)->second.get();
49-
50-
return nullptr;
44+
std::lock_guard lock(m_pagesMutex);
45+
return getPage(adr) != nullptr;
5146
}
5247
void addHook(uintptr_t adr, hl::Hooker::HookCallback_t cbHook)
5348
{
@@ -63,6 +58,7 @@ class VEHHookManager
6358
}
6459
}
6560

61+
std::lock_guard lock(m_pagesMutex);
6662
auto page = getPage(adr);
6763
if (page)
6864
{
@@ -74,42 +70,45 @@ class VEHHookManager
7470
uintptr_t lowerBound = region.base;
7571
uintptr_t upperBound = lowerBound + hl::GetPageSize();
7672

77-
std::lock_guard<std::mutex> lock(m_pagesMutex);
7873
m_pages[lowerBound] = std::make_unique<Page>(lowerBound, upperBound);
79-
m_pages[upperBound] = nullptr;
74+
m_pages.try_emplace(upperBound, nullptr);
8075
}
8176
}
8277
void removeHook(uintptr_t adr)
8378
{
8479
m_hooks.erase(adr);
8580

86-
auto page = getPage(adr);
87-
if (page)
8881
{
89-
if (page->m_refs == 1)
82+
std::unique_lock lock(m_pagesMutex);
83+
auto page = getPage(adr);
84+
if (page)
9085
{
91-
// Trigger the guard page violation to remove it. VEH will not handle the
92-
// exception so we can be sure the guard page protection was removed.
93-
[&] {
94-
__try
86+
if (page->m_refs == 1)
87+
{
88+
lock.unlock();
89+
// Trigger the guard page violation to remove it. VEH will not handle the
90+
// exception so we can be sure the guard page protection was removed.
91+
[&]
9592
{
96-
while (true)
93+
__try
9794
{
98-
auto x = *(volatile int*)adr;
95+
while (true)
96+
{
97+
auto x = *(volatile int*)adr;
98+
}
9999
}
100-
}
101-
__except (EXCEPTION_EXECUTE_HANDLER)
102-
{
103-
}
104-
}();
105-
106-
std::lock_guard<std::mutex> lock(m_pagesMutex);
107-
m_pages.erase(page->m_end);
108-
m_pages.erase(page->m_begin);
109-
}
110-
else
111-
{
112-
page->m_refs--;
100+
__except (EXCEPTION_EXECUTE_HANDLER)
101+
{
102+
}
103+
}();
104+
105+
lock.lock();
106+
m_pages.erase(page->m_begin);
107+
}
108+
else
109+
{
110+
page->m_refs--;
111+
}
113112
}
114113
}
115114

@@ -121,6 +120,17 @@ class VEHHookManager
121120
}
122121
}
123122

123+
private:
124+
// Caller must lock.
125+
Page* getPage(uintptr_t adr)
126+
{
127+
auto it = m_pages.upper_bound(adr);
128+
if (it != m_pages.end())
129+
return (--it)->second.get();
130+
131+
return nullptr;
132+
}
133+
124134
private:
125135
PVOID m_pExHandler = nullptr;
126136
std::map<uintptr_t, hl::Hooker::HookCallback_t> m_hooks;
@@ -243,7 +253,7 @@ static void checkForHookAndCall(uintptr_t adr, CONTEXT* pCtx)
243253

244254
static LONG CALLBACK VectoredHandler(PEXCEPTION_POINTERS exc)
245255
{
246-
static uintptr_t guardFaultAdr;
256+
thread_local uintptr_t guardFaultAdr;
247257

248258
CONTEXT* pCtx = exc->ContextRecord;
249259

@@ -285,7 +295,7 @@ static LONG CALLBACK VectoredHandler(PEXCEPTION_POINTERS exc)
285295
// Single-step exeption occured. Single-step flag is cleared now.
286296

287297
// Check if we are within a page that contains hooks.
288-
if (g_vehHookManager.getPage(pCtx->REG_INSTRUCTIONPTR))
298+
if (g_vehHookManager.isPageHooked(pCtx->REG_INSTRUCTIONPTR))
289299
{
290300
// We are still on a hooked page. Continue single-stepping.
291301
pCtx->EFlags |= 0x100;
@@ -296,7 +306,7 @@ static LONG CALLBACK VectoredHandler(PEXCEPTION_POINTERS exc)
296306
{
297307
// We stepped out of a hooked page.
298308
// Check if the page that was last guarded still contains a hook.
299-
if (g_vehHookManager.getPage(guardFaultAdr))
309+
if (g_vehHookManager.isPageHooked(guardFaultAdr))
300310
{
301311
// Restore guard page protection.
302312
DWORD oldProt;

‎src/hacklib/src/Logging.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ static std::string GetCodeStr(const char* file, const char* func, int line)
5959
return str;
6060
}
6161

62-
static void LogString(const std::string& str)
62+
static void LogString(const std::string& str, bool raw)
6363
{
6464
std::string logStr = str;
65-
if (g_cfg.logTime)
65+
if (!raw && g_cfg.logTime)
6666
{
6767
logStr = GetTimeStr() + " " + str;
6868
}
@@ -97,7 +97,7 @@ void hl::LogDebug(const char* file, const char* func, int line, const char* form
9797
va_start(vl, format);
9898
FormatStr str(format, vl);
9999

100-
LogString(GetCodeStr(file, func, line) + " " + str.str());
100+
LogString(GetCodeStr(file, func, line) + " " + str.str(), false);
101101

102102
va_end(vl);
103103
}
@@ -108,7 +108,7 @@ void hl::LogError(const char* file, const char* func, int line, const char* form
108108
va_start(vl, format);
109109
FormatStr str(format, vl);
110110

111-
LogString(GetCodeStr(file, func, line) + " ERROR: " + str.str());
111+
LogString(GetCodeStr(file, func, line) + " ERROR: " + str.str(), false);
112112

113113
va_end(vl);
114114
}
@@ -119,7 +119,7 @@ void hl::LogError(const char* format, ...)
119119
va_start(vl, format);
120120
FormatStr str(format, vl);
121121

122-
LogString(std::string("ERROR: ") + str.str());
122+
LogString(std::string("ERROR: ") + str.str(), false);
123123

124124
va_end(vl);
125125
}
@@ -130,7 +130,7 @@ void hl::LogRaw(const char* format, ...)
130130
va_start(vl, format);
131131
FormatStr str(format, vl);
132132

133-
LogString(str.str());
133+
LogString(str.str(), true);
134134

135135
va_end(vl);
136136
}

0 commit comments

Comments
 (0)