From c1a5bf9248f55df99dc50420e0a119152c911d5c Mon Sep 17 00:00:00 2001 From: chrb22 Date: Sat, 18 Mar 2023 22:29:13 +0100 Subject: [PATCH 01/14] Make post-hook creation optional --- extensions/dhooks/DynamicHooks/hook.cpp | 15 +++++++++------ extensions/dhooks/DynamicHooks/hook.h | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/extensions/dhooks/DynamicHooks/hook.cpp b/extensions/dhooks/DynamicHooks/hook.cpp index dd25524858..b925cb22c6 100644 --- a/extensions/dhooks/DynamicHooks/hook.cpp +++ b/extensions/dhooks/DynamicHooks/hook.cpp @@ -85,8 +85,8 @@ CHook::CHook(void* pFunc, ICallingConvention* pConvention) // Save the trampoline m_pTrampoline = (void *) pCopiedBytes; - // Create the bridge function - m_pBridge = CreateBridge(); + // Create the bridge function. A post hook can't be created if it isn't a function call and won't have a stack argument pointer. + m_pBridge = CreateBridge(pConvention->GetStackArgumentPtr(m_pRegisters) != NULL); // Write a jump to the bridge DoGatePatch((unsigned char *) pFunc, m_pBridge); @@ -102,7 +102,8 @@ CHook::~CHook() // Free the asm bridge and new return address smutils->GetScriptingEngine()->FreePageMemory(m_pBridge); - smutils->GetScriptingEngine()->FreePageMemory(m_pNewRetAddr); + if (m_pNewRetAddr) + smutils->GetScriptingEngine()->FreePageMemory(m_pNewRetAddr); delete m_pRegisters; delete m_pCallingConvention; @@ -229,13 +230,14 @@ void __cdecl CHook::SetReturnAddress(void* pRetAddr, void* pESP) i->value.push_back(pRetAddr); } -void* CHook::CreateBridge() +void* CHook::CreateBridge(bool createPostHook) { sp::MacroAssembler masm; Label label_supercede; // Write a redirect to the post-hook code - Write_ModifyReturnAddress(masm); + if (createPostHook) + Write_ModifyReturnAddress(masm); // Call the pre-hook handler and jump to label_supercede if ReturnAction_Supercede was returned Write_CallHandler(masm, HOOKTYPE_PRE); @@ -244,7 +246,8 @@ void* CHook::CreateBridge() // Restore the previously saved registers, so any changes will be applied Write_RestoreRegisters(masm, HOOKTYPE_PRE); - masm.j(equal, &label_supercede); + if (createPostHook) + masm.j(equal, &label_supercede); // Jump to the trampoline masm.jmp(ExternalAddress(m_pTrampoline)); diff --git a/extensions/dhooks/DynamicHooks/hook.h b/extensions/dhooks/DynamicHooks/hook.h index d7742d7a5b..7066b45dce 100644 --- a/extensions/dhooks/DynamicHooks/hook.h +++ b/extensions/dhooks/DynamicHooks/hook.h @@ -173,7 +173,7 @@ class CHook } private: - void* CreateBridge(); + void* CreateBridge(bool createPostHook); void Write_ModifyReturnAddress(sp::MacroAssembler& masm); void Write_CallHandler(sp::MacroAssembler& masm, HookType_t type); From 154ceb26d1d67ba217aa69a254e63c5dcf2ba8a7 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Sat, 18 Mar 2023 22:37:54 +0100 Subject: [PATCH 02/14] Add instruction "calling convention" --- extensions/dhooks/AMBuilder | 1 + .../conventions/x86Instruction.cpp | 117 ++++++++++++++++++ .../DynamicHooks/conventions/x86Instruction.h | 68 ++++++++++ extensions/dhooks/dynhooks_sourcepawn.cpp | 16 +++ extensions/dhooks/signatures.cpp | 2 + extensions/dhooks/vhook.h | 1 + plugins/include/dhooks.inc | 1 + 7 files changed, 206 insertions(+) create mode 100644 extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp create mode 100644 extensions/dhooks/DynamicHooks/conventions/x86Instruction.h diff --git a/extensions/dhooks/AMBuilder b/extensions/dhooks/AMBuilder index 151b922560..7513ac8b91 100644 --- a/extensions/dhooks/AMBuilder +++ b/extensions/dhooks/AMBuilder @@ -54,6 +54,7 @@ for cxx in builder.targets: os.path.join('DynamicHooks', 'conventions', 'x86MsCdecl.cpp'), os.path.join('DynamicHooks', 'conventions', 'x86MsStdcall.cpp'), os.path.join('DynamicHooks', 'conventions', 'x86MsFastcall.cpp'), + os.path.join('DynamicHooks', 'conventions', 'x86Instruction.cpp'), ] if binary.compiler.target.platform == 'windows': diff --git a/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp new file mode 100644 index 0000000000..9cdb8e637c --- /dev/null +++ b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp @@ -0,0 +1,117 @@ +/** +* ============================================================================= +* DynamicHooks +* Copyright (C) 2015 Robin Gohmert. All rights reserved. +* Copyright (C) 2018-2021 AlliedModders LLC. All rights reserved. +* ============================================================================= +* +* This software is provided 'as-is', without any express or implied warranty. +* In no event will the authors be held liable for any damages arising from +* the use of this software. +* +* Permission is granted to anyone to use this software for any purpose, +* including commercial applications, and to alter it and redistribute it +* freely, subject to the following restrictions: +* +* 1. The origin of this software must not be misrepresented; you must not +* claim that you wrote the original software. If you use this software in a +* product, an acknowledgment in the product documentation would be +* appreciated but is not required. +* +* 2. Altered source versions must be plainly marked as such, and must not be +* misrepresented as being the original software. +* +* 3. This notice may not be removed or altered from any source distribution. +* +* asm.h/cpp from devmaster.net (thanks cybermind) edited by pRED* to handle gcc +* -fPIC thunks correctly +* +* Idea and trampoline code taken from DynDetours (thanks your-name-here). +* +* Adopted to provide similar features to SourceHook by AlliedModders LLC. +*/ + +// ============================================================================ +// >> INCLUDES +// ============================================================================ +#include "x86Instruction.h" +#include + + +// ============================================================================ +// >> x86Instruction +// ============================================================================ +x86Instruction::x86Instruction(std::vector &vecArgTypes, DataTypeSized_t returnType, int iAlignment) : + ICallingConvention(vecArgTypes, returnType, iAlignment) +{ +} + +x86Instruction::~x86Instruction() +{ +} + +std::vector x86Instruction::GetRegisters() +{ + std::vector registers; + + // Save all the custom calling convention registers as well. + for (size_t i = 0; i < m_vecArgTypes.size(); i++) + { + if (m_vecArgTypes[i].custom_register == None) + continue; + + // TODO: Make sure the list is unique? Set? + registers.push_back(m_vecArgTypes[i].custom_register); + } + + return registers; +} + +int x86Instruction::GetPopSize() +{ + return 0; +} + +int x86Instruction::GetArgStackSize() +{ + return 0; +} + +void** x86Instruction::GetStackArgumentPtr(CRegisters* pRegisters) +{ + return NULL; +} + +int x86Instruction::GetArgRegisterSize() +{ + return 0; +} + +void* x86Instruction::GetArgumentPtr(unsigned int iIndex, CRegisters* pRegisters) +{ + if (iIndex >= m_vecArgTypes.size()) + return NULL; + + // Check if this argument wasn't passed in a register. + if (m_vecArgTypes[iIndex].custom_register == None) + return NULL; + + CRegister *pRegister = pRegisters->GetRegister(m_vecArgTypes[iIndex].custom_register); + if (!pRegister) + return NULL; + + return pRegister->m_pAddress; +} + +void x86Instruction::ArgumentPtrChanged(unsigned int iIndex, CRegisters* pRegisters, void* pArgumentPtr) +{ +} + +void* x86Instruction::GetReturnPtr(CRegisters* pRegisters) +{ + return NULL; +} + +void x86Instruction::ReturnPtrChanged(CRegisters* pRegisters, void* pReturnPtr) +{ +} diff --git a/extensions/dhooks/DynamicHooks/conventions/x86Instruction.h b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.h new file mode 100644 index 0000000000..daff070384 --- /dev/null +++ b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.h @@ -0,0 +1,68 @@ +/** +* ============================================================================= +* DynamicHooks +* Copyright (C) 2015 Robin Gohmert. All rights reserved. +* Copyright (C) 2018-2021 AlliedModders LLC. All rights reserved. +* ============================================================================= +* +* This software is provided 'as-is', without any express or implied warranty. +* In no event will the authors be held liable for any damages arising from +* the use of this software. +* +* Permission is granted to anyone to use this software for any purpose, +* including commercial applications, and to alter it and redistribute it +* freely, subject to the following restrictions: +* +* 1. The origin of this software must not be misrepresented; you must not +* claim that you wrote the original software. If you use this software in a +* product, an acknowledgment in the product documentation would be +* appreciated but is not required. +* +* 2. Altered source versions must be plainly marked as such, and must not be +* misrepresented as being the original software. +* +* 3. This notice may not be removed or altered from any source distribution. +* +* asm.h/cpp from devmaster.net (thanks cybermind) edited by pRED* to handle gcc +* -fPIC thunks correctly +* +* Idea and trampoline code taken from DynDetours (thanks your-name-here). +* +* Adopted to provide similar features to SourceHook by AlliedModders LLC. +*/ + +#ifndef _X86_INSTRUCTION_H +#define _X86_INSTRUCTION_H + +// ============================================================================ +// >> INCLUDES +// ============================================================================ +#include "../convention.h" + + +// ============================================================================ +// >> CLASSES +// ============================================================================ +class x86Instruction: public ICallingConvention +{ +public: + x86Instruction(std::vector &vecArgTypes, DataTypeSized_t returnType, int iAlignment=4); + virtual ~x86Instruction(); + + virtual std::vector GetRegisters(); + virtual int GetPopSize(); + virtual int GetArgStackSize(); + virtual void** GetStackArgumentPtr(CRegisters* pRegisters); + virtual int GetArgRegisterSize(); + + virtual void* GetArgumentPtr(unsigned int iIndex, CRegisters* pRegisters); + virtual void ArgumentPtrChanged(unsigned int iIndex, CRegisters* pRegisters, void* pArgumentPtr); + + virtual void* GetReturnPtr(CRegisters* pRegisters); + virtual void ReturnPtrChanged(CRegisters* pRegisters, void* pReturnPtr); + +private: + void* m_pReturnBuffer; +}; + +#endif // _X86_INSTRUCTION_H diff --git a/extensions/dhooks/dynhooks_sourcepawn.cpp b/extensions/dhooks/dynhooks_sourcepawn.cpp index 8b148fe7d8..23bf0c25e9 100644 --- a/extensions/dhooks/dynhooks_sourcepawn.cpp +++ b/extensions/dhooks/dynhooks_sourcepawn.cpp @@ -38,21 +38,25 @@ #include "conventions/x86MsThiscall.h" #include "conventions/x86MsStdcall.h" #include "conventions/x86MsFastcall.h" +#include "conventions/x86Instruction.h" typedef x86MsCdecl x86DetourCdecl; typedef x86MsThiscall x86DetourThisCall; typedef x86MsStdcall x86DetourStdCall; typedef x86MsFastcall x86DetourFastCall; +typedef x86Instruction x86DetourInstruction; #elif defined KE_LINUX #include "conventions/x86GccCdecl.h" #include "conventions/x86GccThiscall.h" #include "conventions/x86MsStdcall.h" #include "conventions/x86MsFastcall.h" +#include typedef x86GccCdecl x86DetourCdecl; typedef x86GccThiscall x86DetourThisCall; // Uhm, stdcall on linux? typedef x86MsStdcall x86DetourStdCall; // Uhumm, fastcall on linux? typedef x86MsFastcall x86DetourFastCall; +typedef x86Instruction x86DetourInstruction; #else #error "Unsupported platform." #endif @@ -241,6 +245,10 @@ ICallingConvention *ConstructCallingConvention(HookSetup *setup) break; case CallConv_FASTCALL: pCallConv = new x86DetourFastCall(vecArgTypes, returnType); + break; + case CallConv_INSTRUCTION: + pCallConv = new x86DetourInstruction(vecArgTypes, returnType); + break; default: smutils->LogError(myself, "Unknown calling convention %d.", setup->callConv); @@ -424,6 +432,14 @@ ReturnAction_t HandleDetour(HookType_t hookType, CHook* pDetour) } } + // It doesn't make sense to supercede instruction detours when they're detouring individual instructions + if (pWrapper->callConv == CallConv_INSTRUCTION && result == MRES_Supercede) + { + tempRet = ReturnAction_Ignored; + pCallback->GetParentRuntime()->GetDefaultContext()->BlamePluginError(pCallback, "Tried to supercede an instruction detour which is not possible"); + break; + } + // Store if the plugin wants the original function to be called. if (result == MRES_Supercede) tempRet = ReturnAction_Supercede; diff --git a/extensions/dhooks/signatures.cpp b/extensions/dhooks/signatures.cpp index 45b673cc30..f757778243 100644 --- a/extensions/dhooks/signatures.cpp +++ b/extensions/dhooks/signatures.cpp @@ -223,6 +223,8 @@ SMCResult SignatureGameConfig::ReadSMC_KeyValue(const SMCStates *states, const c callConv = CallConv_STDCALL; else if (!strcmp(value, "fastcall")) callConv = CallConv_FASTCALL; + else if (!strcmp(value, "instruction")) + callConv = CallConv_INSTRUCTION; else { smutils->LogError(myself, "Invalid calling convention \"%s\": line: %i col: %i", value, states->line, states->col); diff --git a/extensions/dhooks/vhook.h b/extensions/dhooks/vhook.h index 6146d1ad6a..2bfb61014e 100644 --- a/extensions/dhooks/vhook.h +++ b/extensions/dhooks/vhook.h @@ -45,6 +45,7 @@ enum CallingConvention CallConv_THISCALL, CallConv_STDCALL, CallConv_FASTCALL, + CallConv_INSTRUCTION, }; enum MRESReturn diff --git a/plugins/include/dhooks.inc b/plugins/include/dhooks.inc index 76ff8c4de3..5d4f5bc94b 100644 --- a/plugins/include/dhooks.inc +++ b/plugins/include/dhooks.inc @@ -115,6 +115,7 @@ enum CallingConvention CallConv_THISCALL, CallConv_STDCALL, CallConv_FASTCALL, + CallConv_INSTRUCTION }; enum HookMode From 45020bd634dcbb55f68c2b22d5a4b3e683142a58 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Sat, 18 Mar 2023 22:39:18 +0100 Subject: [PATCH 03/14] Add errors to instruction hook setup --- extensions/dhooks/natives.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/extensions/dhooks/natives.cpp b/extensions/dhooks/natives.cpp index 8757e8319d..742be6d8e7 100644 --- a/extensions/dhooks/natives.cpp +++ b/extensions/dhooks/natives.cpp @@ -101,6 +101,9 @@ IPluginFunction *GetCallback(IPluginContext *pContext, HookSetup * setup, const //native Handle:DHookCreate(offset, HookType:hooktype, ReturnType:returntype, ThisPointerType:thistype, DHookCallback:callback = INVALID_FUNCTION); // Callback is now optional here. cell_t Native_CreateHook(IPluginContext *pContext, const cell_t *params) { + if ((CallingConvention)params[2] == CallConv_INSTRUCTION) + return pContext->ThrowNativeError("Can't create an instruction hook from a vtable offset."); + IPluginFunction *callback = nullptr; // The methodmap constructor doesn't have the callback parameter anymore. if (params[0] >= 5) @@ -122,6 +125,15 @@ cell_t Native_CreateHook(IPluginContext *pContext, const cell_t *params) //native Handle:DHookCreateDetour(Address:funcaddr, CallingConvention:callConv, ReturnType:returntype, ThisPointerType:thistype); cell_t Native_CreateDetour(IPluginContext *pContext, const cell_t *params) { + if ((CallingConvention)params[2] == CallConv_INSTRUCTION) + { + if ((ReturnType)params[3] != ReturnType_Void) + return pContext->ThrowNativeError("Return type must be void for an instruction hook."); + + if ((ThisPointerType)params[4] != ThisPointer_Ignore) + return pContext->ThrowNativeError("This pointer must be ignored for an instruction hook."); + } + HookSetup *setup = new HookSetup((ReturnType)params[3], PASSFLAG_BYVAL, (CallingConvention)params[2], (ThisPointerType)params[4], (void *)params[1]); Handle_t hndl = handlesys->CreateHandle(g_HookSetupHandle, setup, pContext->GetIdentity(), myself->GetIdentity(), NULL); @@ -185,6 +197,18 @@ cell_t Native_DHookCreateFromConf(IPluginContext *pContext, const cell_t *params } } + if (sig->callConv == CallConv_INSTRUCTION) + { + if (sig->retType != ReturnType_Void) + return pContext->ThrowNativeError("Return type must be void for an instruction hook."); + + for (ArgumentInfo &arg : sig->args) + { + if (arg.info.custom_register != None) + return pContext->ThrowNativeError("Must specify registers for parameters in an instruction hook."); + } + } + setup = new HookSetup(sig->retType, PASSFLAG_BYVAL, sig->callConv, sig->thisType, addr); } @@ -315,6 +339,9 @@ cell_t Native_AddParam(IPluginContext *pContext, const cell_t *params) info.custom_register = None; } + if (setup->callConv == CallConv_INSTRUCTION && info.custom_register == None) + return pContext->ThrowNativeError("Must specify registers for parameters in an instruction hook."); + if(params[0] >= 3 && params[3] != -1) { info.size = params[3]; @@ -359,6 +386,9 @@ cell_t Native_EnableDetour(IPluginContext *pContext, const cell_t *params) bool post = params[2] != 0; HookType_t hookType = post ? HOOKTYPE_POST : HOOKTYPE_PRE; + if (setup->callConv == CallConv_INSTRUCTION && hookType == HOOKTYPE_POST) + return pContext->ThrowNativeError("Can't create a post hook for instruction hooks."); + // Check if we already detoured that function. CHookManager *pDetourManager = GetHookManager(); CHook* pDetour = pDetourManager->FindHook(setup->funcAddr); From 7ec67d986c809b78481f62b0487e94e69f023ce4 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Thu, 30 Mar 2023 20:05:03 +0200 Subject: [PATCH 04/14] Initialize `m_pNewRetAddr` as `NULL` --- extensions/dhooks/DynamicHooks/hook.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/dhooks/DynamicHooks/hook.cpp b/extensions/dhooks/DynamicHooks/hook.cpp index b925cb22c6..b774453c3e 100644 --- a/extensions/dhooks/DynamicHooks/hook.cpp +++ b/extensions/dhooks/DynamicHooks/hook.cpp @@ -57,6 +57,7 @@ CHook::CHook(void* pFunc, ICallingConvention* pConvention) m_pFunc = pFunc; m_pRegisters = new CRegisters(pConvention->GetRegisters()); m_pCallingConvention = pConvention; + m_pNewRetAddr = NULL; if (!m_hookHandler.init()) return; From 657b3dded382433c0003c6f68bd1c00c69bbffa5 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Fri, 31 Mar 2023 16:26:13 +0200 Subject: [PATCH 05/14] Remove wrongly created error check --- extensions/dhooks/natives.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/extensions/dhooks/natives.cpp b/extensions/dhooks/natives.cpp index 742be6d8e7..00fa524811 100644 --- a/extensions/dhooks/natives.cpp +++ b/extensions/dhooks/natives.cpp @@ -101,9 +101,6 @@ IPluginFunction *GetCallback(IPluginContext *pContext, HookSetup * setup, const //native Handle:DHookCreate(offset, HookType:hooktype, ReturnType:returntype, ThisPointerType:thistype, DHookCallback:callback = INVALID_FUNCTION); // Callback is now optional here. cell_t Native_CreateHook(IPluginContext *pContext, const cell_t *params) { - if ((CallingConvention)params[2] == CallConv_INSTRUCTION) - return pContext->ThrowNativeError("Can't create an instruction hook from a vtable offset."); - IPluginFunction *callback = nullptr; // The methodmap constructor doesn't have the callback parameter anymore. if (params[0] >= 5) From 42b1e5ec03c538288ab26ea8a5e166e11144c62f Mon Sep 17 00:00:00 2001 From: chrb22 Date: Fri, 31 Mar 2023 16:56:12 +0200 Subject: [PATCH 06/14] Add checks for instruction hooks in gamedata parsing --- extensions/dhooks/natives.cpp | 12 ------------ extensions/dhooks/signatures.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/extensions/dhooks/natives.cpp b/extensions/dhooks/natives.cpp index 00fa524811..9df982b871 100644 --- a/extensions/dhooks/natives.cpp +++ b/extensions/dhooks/natives.cpp @@ -194,18 +194,6 @@ cell_t Native_DHookCreateFromConf(IPluginContext *pContext, const cell_t *params } } - if (sig->callConv == CallConv_INSTRUCTION) - { - if (sig->retType != ReturnType_Void) - return pContext->ThrowNativeError("Return type must be void for an instruction hook."); - - for (ArgumentInfo &arg : sig->args) - { - if (arg.info.custom_register != None) - return pContext->ThrowNativeError("Must specify registers for parameters in an instruction hook."); - } - } - setup = new HookSetup(sig->retType, PASSFLAG_BYVAL, sig->callConv, sig->thisType, addr); } diff --git a/extensions/dhooks/signatures.cpp b/extensions/dhooks/signatures.cpp index f757778243..6dbb56d370 100644 --- a/extensions/dhooks/signatures.cpp +++ b/extensions/dhooks/signatures.cpp @@ -231,6 +231,21 @@ SMCResult SignatureGameConfig::ReadSMC_KeyValue(const SMCStates *states, const c return SMCResult_HaltFail; } + if (callConv == CallConv_INSTRUCTION) + { + if (g_CurrentSignature->thisType != ThisPointer_Ignore) + { + smutils->LogError(myself, "Cannot use a this pointer with an instruction hook: line: %i col: %i", states->line, states->col); + return SMCResult_HaltFail; + } + + if (g_CurrentSignature->retType != ReturnType_Unknown && g_CurrentSignature->retType != ReturnType_Void) + { + smutils->LogError(myself, "Cannot use a non-void return type with an instruction hook: line: %i col: %i", states->line, states->col); + return SMCResult_HaltFail; + } + } + g_CurrentSignature->callConv = callConv; } else if (!strcmp(key, "hooktype")) @@ -260,6 +275,12 @@ SMCResult SignatureGameConfig::ReadSMC_KeyValue(const SMCStates *states, const c smutils->LogError(myself, "Invalid return type \"%s\": line: %i col: %i", value, states->line, states->col); return SMCResult_HaltFail; } + + if (g_CurrentSignature->callConv == CallConv_INSTRUCTION && g_CurrentSignature->retType != ReturnType_Void) + { + smutils->LogError(myself, "Cannot use a non-void return type with an instruction hook: line: %i col: %i", states->line, states->col); + return SMCResult_HaltFail; + } } else if (!strcmp(key, "this")) { @@ -274,6 +295,12 @@ SMCResult SignatureGameConfig::ReadSMC_KeyValue(const SMCStates *states, const c smutils->LogError(myself, "Invalid this type \"%s\": line: %i col: %i", value, states->line, states->col); return SMCResult_HaltFail; } + + if (g_CurrentSignature->callConv == CallConv_INSTRUCTION && g_CurrentSignature->thisType != ThisPointer_Ignore) + { + smutils->LogError(myself, "Cannot use a this pointer with an instruction hook: line: %i col: %i", states->line, states->col); + return SMCResult_HaltFail; + } } else { From e90691739b14337a0b4d51ef3b4ffeb1908d4caf Mon Sep 17 00:00:00 2001 From: chrb22 Date: Fri, 31 Mar 2023 17:05:23 +0200 Subject: [PATCH 07/14] Add parameter check back --- extensions/dhooks/signatures.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/extensions/dhooks/signatures.cpp b/extensions/dhooks/signatures.cpp index 6dbb56d370..dcd50bc383 100644 --- a/extensions/dhooks/signatures.cpp +++ b/extensions/dhooks/signatures.cpp @@ -436,6 +436,12 @@ SMCResult SignatureGameConfig::ReadSMC_LeavingSection(const SMCStates *states) return SMCResult_HaltFail; } + if (g_CurrentSignature->callConv == CallConv_INSTRUCTION && g_CurrentArgumentInfo.info.custom_register == None) + { + smutils->LogError(myself, "Must specify registers for parameters in an instruction hook: line: %i col: %i", states->line, states->col); + return SMCResult_HaltFail; + } + // The size wasn't set in the config. See if that's fine and we can guess it from the type. if (!g_CurrentArgumentInfo.info.size) { From 19f15c0808633bb9e77c3b3119979c5460da61a8 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Fri, 31 Mar 2023 17:58:52 +0200 Subject: [PATCH 08/14] Save all registers in instruction hooks --- .../DynamicHooks/conventions/x86Instruction.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp index 9cdb8e637c..4657bfa660 100644 --- a/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp +++ b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp @@ -52,17 +52,13 @@ x86Instruction::~x86Instruction() std::vector x86Instruction::GetRegisters() { - std::vector registers; - - // Save all the custom calling convention registers as well. - for (size_t i = 0; i < m_vecArgTypes.size(); i++) + std::vector registers = { - if (m_vecArgTypes[i].custom_register == None) - continue; - - // TODO: Make sure the list is unique? Set? - registers.push_back(m_vecArgTypes[i].custom_register); - } + AL, CL, DL, BL, AH, CH, DH, BH, + EAX, ECX, EDX, EBX, ESP, EBP, ESI, EDI, + XMM0, XMM1, XMM2, XMM3, XMM4, XMM5, XMM6, XMM7, + ST0 + }; return registers; } From ddb906615236158f6428e3de267267f9a238a9e4 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Fri, 31 Mar 2023 21:57:00 +0200 Subject: [PATCH 09/14] Implement `x86Instruction::GetArgRegisterSize` properly --- .../dhooks/DynamicHooks/conventions/x86Instruction.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp index 4657bfa660..38c363e5ca 100644 --- a/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp +++ b/extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp @@ -80,7 +80,15 @@ void** x86Instruction::GetStackArgumentPtr(CRegisters* pRegisters) int x86Instruction::GetArgRegisterSize() { - return 0; + int iArgRegisterSize = 0; + + for (size_t i = 0; i < m_vecArgTypes.size(); i++) + { + if (m_vecArgTypes[i].custom_register != None) + iArgRegisterSize += m_vecArgTypes[i].size; + } + + return iArgRegisterSize; } void* x86Instruction::GetArgumentPtr(unsigned int iIndex, CRegisters* pRegisters) From e0e3cfbef1ebdd01e2b3ca2aa54742f489be6510 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Sat, 1 Apr 2023 18:57:22 +0200 Subject: [PATCH 10/14] Add ability to supercede instruction hook --- extensions/dhooks/DynamicHooks/hook.cpp | 22 +++++++++++++++------- extensions/dhooks/dynhooks_sourcepawn.cpp | 8 -------- public/asm/asm.c | 16 ++++++++++++++++ public/asm/asm.h | 3 +++ 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/extensions/dhooks/DynamicHooks/hook.cpp b/extensions/dhooks/DynamicHooks/hook.cpp index b774453c3e..cd9dd40a3f 100644 --- a/extensions/dhooks/DynamicHooks/hook.cpp +++ b/extensions/dhooks/DynamicHooks/hook.cpp @@ -231,13 +231,13 @@ void __cdecl CHook::SetReturnAddress(void* pRetAddr, void* pESP) i->value.push_back(pRetAddr); } -void* CHook::CreateBridge(bool createPostHook) +void* CHook::CreateBridge(bool isFunctionHook) { sp::MacroAssembler masm; Label label_supercede; // Write a redirect to the post-hook code - if (createPostHook) + if (isFunctionHook) Write_ModifyReturnAddress(masm); // Call the pre-hook handler and jump to label_supercede if ReturnAction_Supercede was returned @@ -247,8 +247,7 @@ void* CHook::CreateBridge(bool createPostHook) // Restore the previously saved registers, so any changes will be applied Write_RestoreRegisters(masm, HOOKTYPE_PRE); - if (createPostHook) - masm.j(equal, &label_supercede); + masm.j(equal, &label_supercede); // Jump to the trampoline masm.jmp(ExternalAddress(m_pTrampoline)); @@ -256,9 +255,18 @@ void* CHook::CreateBridge(bool createPostHook) // This code will be executed if a pre-hook returns ReturnAction_Supercede masm.bind(&label_supercede); - // Finally, return to the caller - // This will still call post hooks, but will skip the original function. - masm.ret(m_pCallingConvention->GetPopSize()); + if (isFunctionHook) + { + // Finally, return to the caller + // This will still call post hooks, but will skip the original function. + masm.ret(m_pCallingConvention->GetPopSize()); + } + else + { + // Jump to the instruction right after the hooked instruction inside the trampoline + void* target = (char*)m_pTrampoline + instruction_length(m_pFunc); + masm.jmp(ExternalAddress(target)); + } void *base = smutils->GetScriptingEngine()->AllocatePageMemory(masm.length()); masm.emitToExecutableMemory(base); diff --git a/extensions/dhooks/dynhooks_sourcepawn.cpp b/extensions/dhooks/dynhooks_sourcepawn.cpp index 23bf0c25e9..4cd38a55de 100644 --- a/extensions/dhooks/dynhooks_sourcepawn.cpp +++ b/extensions/dhooks/dynhooks_sourcepawn.cpp @@ -432,14 +432,6 @@ ReturnAction_t HandleDetour(HookType_t hookType, CHook* pDetour) } } - // It doesn't make sense to supercede instruction detours when they're detouring individual instructions - if (pWrapper->callConv == CallConv_INSTRUCTION && result == MRES_Supercede) - { - tempRet = ReturnAction_Ignored; - pCallback->GetParentRuntime()->GetDefaultContext()->BlamePluginError(pCallback, "Tried to supercede an instruction detour which is not possible"); - break; - } - // Store if the plugin wants the original function to be called. if (result == MRES_Supercede) tempRet = ReturnAction_Supercede; diff --git a/public/asm/asm.c b/public/asm/asm.c index 44337f7e3a..8e3bf8daa6 100644 --- a/public/asm/asm.c +++ b/public/asm/asm.c @@ -89,6 +89,22 @@ void check_thunks(unsigned char *dest, unsigned char *pc) #endif } +int instruction_length(void *addr) +{ + ud_t ud_obj; + ud_init(&ud_obj); + +#if defined(_WIN64) || defined(__x86_64__) + ud_set_mode(&ud_obj, 64); +#else + ud_set_mode(&ud_obj, 32); +#endif + + ud_set_input_buffer(&ud_obj, addr, 20); + + return ud_disassemble(&ud_obj) ? ud_insn_len(&ud_obj) : -1; +} + int copy_bytes(unsigned char *func, unsigned char *dest, unsigned int required_len) { ud_t ud_obj; diff --git a/public/asm/asm.h b/public/asm/asm.h index d61ff30f24..a404c77b34 100644 --- a/public/asm/asm.h +++ b/public/asm/asm.h @@ -19,6 +19,9 @@ extern "C" { void check_thunks(unsigned char *dest, unsigned char *pc); +//get the length of an instruction in bytes +int instruction_length(void *addr); + //if dest is NULL, returns minimum number of bytes needed to be copied //if dest is not NULL, it will copy the bytes to dest as well as fix CALLs and JMPs //http://www.devmaster.net/forums/showthread.php?t=2311 From abba523fd25d0c85847ae47cb6a64dfb3852512b Mon Sep 17 00:00:00 2001 From: chrb22 Date: Sat, 1 Apr 2023 20:09:36 +0200 Subject: [PATCH 11/14] Add jump target warning to include file --- plugins/include/dhooks.inc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/include/dhooks.inc b/plugins/include/dhooks.inc index 5d4f5bc94b..08c41a93e2 100644 --- a/plugins/include/dhooks.inc +++ b/plugins/include/dhooks.inc @@ -115,7 +115,10 @@ enum CallingConvention CallConv_THISCALL, CallConv_STDCALL, CallConv_FASTCALL, - CallConv_INSTRUCTION + CallConv_INSTRUCTION, /**< Single instruction pre-hook. Be careful with instructions that are less than 6 bytes in length. + If the instruction length is less than 6 bytes, then the next instruction(s) will be modified too. + If any instruction within 6 bytes (excluding the hooked instruction) is a jump target, then don't hook. + In the graph view of IDA/Ghidra (default colors) there shouldn't be a blue or red line that leads to a node that is labeled (loc_/LAB_). */ }; enum HookMode From eb780f005825fe5542c7532a3b1f15dd2dd39480 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Wed, 19 Apr 2023 19:46:59 +0200 Subject: [PATCH 12/14] Add constructor for `SignatureWrapper` --- extensions/dhooks/signatures.cpp | 4 ++-- extensions/dhooks/signatures.h | 12 ++++++++++++ extensions/dhooks/vhook.h | 2 ++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/extensions/dhooks/signatures.cpp b/extensions/dhooks/signatures.cpp index dcd50bc383..c7b56234b1 100644 --- a/extensions/dhooks/signatures.cpp +++ b/extensions/dhooks/signatures.cpp @@ -213,7 +213,7 @@ SMCResult SignatureGameConfig::ReadSMC_KeyValue(const SMCStates *states, const c } else if (!strcmp(key, "callconv")) { - CallingConvention callConv; + CallingConvention callConv = CallConv_UNKNOWN; if (!strcmp(value, "cdecl")) callConv = CallConv_CDECL; @@ -250,7 +250,7 @@ SMCResult SignatureGameConfig::ReadSMC_KeyValue(const SMCStates *states, const c } else if (!strcmp(key, "hooktype")) { - HookType hookType; + HookType hookType = HookType_Unknown; if (!strcmp(value, "entity")) hookType = HookType_Entity; diff --git a/extensions/dhooks/signatures.h b/extensions/dhooks/signatures.h index de9ce7cd0d..ccbe3d514d 100644 --- a/extensions/dhooks/signatures.h +++ b/extensions/dhooks/signatures.h @@ -59,6 +59,18 @@ class SignatureWrapper { HookType hookType; ReturnType retType; ThisPointerType thisType; + + SignatureWrapper() + { + signature = ""; + address = ""; + offset = ""; + args = std::vector(); + callConv = CallConv_UNKNOWN; + hookType = HookType_Unknown; + retType = ReturnType_Unknown; + thisType = ThisPointer_Ignore; + } }; class SignatureGameConfig : public ITextListener_SMC { diff --git a/extensions/dhooks/vhook.h b/extensions/dhooks/vhook.h index 2bfb61014e..42aa845cff 100644 --- a/extensions/dhooks/vhook.h +++ b/extensions/dhooks/vhook.h @@ -41,6 +41,7 @@ enum CallingConvention { + CallConv_UNKNOWN = -1, CallConv_CDECL, CallConv_THISCALL, CallConv_STDCALL, @@ -116,6 +117,7 @@ enum ThisPointerType enum HookType { + HookType_Unknown = -1, HookType_Entity, HookType_GameRules, HookType_Raw From 5adeef837981212171a511ad32efef71114012a4 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Wed, 19 Apr 2023 19:48:21 +0200 Subject: [PATCH 13/14] Add trailing commas to `dhooks.inc` enums --- plugins/include/dhooks.inc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/include/dhooks.inc b/plugins/include/dhooks.inc index 08c41a93e2..bbc01400de 100644 --- a/plugins/include/dhooks.inc +++ b/plugins/include/dhooks.inc @@ -54,13 +54,13 @@ enum ObjectValueType ObjectValueType_Vector, ObjectValueType_VectorPtr, ObjectValueType_CharPtr, - ObjectValueType_String + ObjectValueType_String, }; enum ListenType { ListenType_Created, - ListenType_Deleted + ListenType_Deleted, }; enum ReturnType @@ -76,7 +76,7 @@ enum ReturnType ReturnType_Vector, ReturnType_VectorPtr, ReturnType_CBaseEntity, - ReturnType_Edict + ReturnType_Edict, }; enum HookParamType @@ -92,21 +92,21 @@ enum HookParamType HookParamType_CBaseEntity, HookParamType_ObjectPtr, HookParamType_Edict, - HookParamType_Object + HookParamType_Object, }; enum ThisPointerType { ThisPointer_Ignore, ThisPointer_CBaseEntity, - ThisPointer_Address + ThisPointer_Address, }; enum HookType { HookType_Entity, HookType_GameRules, - HookType_Raw + HookType_Raw, }; enum CallingConvention @@ -124,7 +124,7 @@ enum CallingConvention enum HookMode { Hook_Pre, /**< Callback will be executed BEFORE the original function. */ - Hook_Post /**< Callback will be executed AFTER the original function. */ + Hook_Post, /**< Callback will be executed AFTER the original function. */ }; enum MRESReturn @@ -134,7 +134,7 @@ enum MRESReturn MRES_Ignored, /**< plugin didn't take any action */ MRES_Handled, /**< plugin did something, but real function should still be called */ MRES_Override, /**< call real function, but use my return value */ - MRES_Supercede /**< skip real function; use my return value */ + MRES_Supercede, /**< skip real function; use my return value */ }; enum DHookPassFlag @@ -182,7 +182,7 @@ enum DHookRegister DHookRegister_XMM7, // 80-bit FPU registers - DHookRegister_ST0 + DHookRegister_ST0, }; typeset ListenCB From 1799c7ec09dac88a9d54fabcc0392af2c1b0ac54 Mon Sep 17 00:00:00 2001 From: chrb22 Date: Thu, 20 Apr 2023 15:55:19 +0200 Subject: [PATCH 14/14] Lower DHooks jump patch size to 5 bytes --- extensions/dhooks/DynamicHooks/hook.cpp | 2 +- plugins/include/dhooks.inc | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/extensions/dhooks/DynamicHooks/hook.cpp b/extensions/dhooks/DynamicHooks/hook.cpp index cd9dd40a3f..4082743466 100644 --- a/extensions/dhooks/DynamicHooks/hook.cpp +++ b/extensions/dhooks/DynamicHooks/hook.cpp @@ -46,7 +46,7 @@ using namespace sp; // ============================================================================ // >> DEFINITIONS // ============================================================================ -#define JMP_SIZE 6 +#define JMP_SIZE 5 // ============================================================================ diff --git a/plugins/include/dhooks.inc b/plugins/include/dhooks.inc index bbc01400de..997af82a5e 100644 --- a/plugins/include/dhooks.inc +++ b/plugins/include/dhooks.inc @@ -115,9 +115,9 @@ enum CallingConvention CallConv_THISCALL, CallConv_STDCALL, CallConv_FASTCALL, - CallConv_INSTRUCTION, /**< Single instruction pre-hook. Be careful with instructions that are less than 6 bytes in length. - If the instruction length is less than 6 bytes, then the next instruction(s) will be modified too. - If any instruction within 6 bytes (excluding the hooked instruction) is a jump target, then don't hook. + CallConv_INSTRUCTION, /**< Single instruction pre-hook. Be careful with instructions that are less than 5 bytes in length. + If the instruction length is less than 5 bytes, then the next instruction(s) will be modified too. + If any instruction within 5 bytes (excluding the hooked instruction) is a jump target, then don't hook. In the graph view of IDA/Ghidra (default colors) there shouldn't be a blue or red line that leads to a node that is labeled (loc_/LAB_). */ }; @@ -571,8 +571,9 @@ methodmap DynamicHook < DHookSetup // with a jump to our own code. This means that the signature used to find // the function address in the first place might not match anymore after a detour. // If you need to detour the same function in different plugins make sure to -// wildcard \x2a the first 6 bytes of the signature to accommodate for the patched -// jump introduced by the detour. +// wildcard \x2a the first 5 bytes of the signature to accommodate for the patched +// jump introduced by the detour. This is not necessary for plugins that use GameConfigs +// because a cached copy of the libraries will be used for finding signatures instead. methodmap DynamicDetour < DHookSetup { // Creates a detour.