diff --git a/UnitTests/linux/TestDetourx86.cpp b/UnitTests/linux/TestDetourx86.cpp index 02e463e..8d87ea6 100644 --- a/UnitTests/linux/TestDetourx86.cpp +++ b/UnitTests/linux/TestDetourx86.cpp @@ -216,8 +216,6 @@ TEST_CASE("Testing x86 detours", "[x86Detour][ADetour]") { REQUIRE(detour.hook() == true); } - // TODO: Fix this. when making jmpToProl, relative displacement can only encode offset up to 0x7FFFFFFF - // But during tests, distance between prologue and trampoline was larger than that, leading to incorrect jump. SECTION("hook printf") { PLH::x86Detour detour((uint64_t)&printf, (uint64_t)h_hookPrintf, &hookPrintfTramp); REQUIRE(detour.hook() == true); diff --git a/UnitTests/linux/TestDisassembler.cpp b/UnitTests/linux/TestDisassembler.cpp index 4b2cd09..d681d4d 100644 --- a/UnitTests/linux/TestDisassembler.cpp +++ b/UnitTests/linux/TestDisassembler.cpp @@ -263,7 +263,7 @@ TEMPLATE_TEST_CASE("Test Disassemblers x86 FF25", "[ZydisDisassembler]", PLH::Zy auto Instructions = disasm.disassemble( (uint64_t)x86ASM_FF25.data(), (uint64_t)x86ASM_FF25.data(), - (uint64_t)(x86ASM_FF25.data() + address_length), + (uint64_t)(jmp_address_ptr + address_length), PLH::MemAccessor() ); diff --git a/UnitTests/windows/TestDetourx64.cpp b/UnitTests/windows/TestDetourx64.cpp index ccfcac1..6b8e0cd 100644 --- a/UnitTests/windows/TestDetourx64.cpp +++ b/UnitTests/windows/TestDetourx64.cpp @@ -9,7 +9,7 @@ #include "polyhook2/PolyHookOsIncludes.hpp" -#include +#include #define WIN32_LEAN_AND_MEAN #include diff --git a/polyhook2/Detour/ADetour.hpp b/polyhook2/Detour/ADetour.hpp index 83a06b7..cb2f9d1 100644 --- a/polyhook2/Detour/ADetour.hpp +++ b/polyhook2/Detour/ADetour.hpp @@ -65,6 +65,17 @@ class Detour : public PLH::IHook { void setIsFollowCallOnFnAddress(bool value); + + /** + * @return instructions of a routine if the provided instruction is a call instruction that calls a routine + * which returns an address stored SP register, which refers to the address of the next instruction following + * the call instruction. An example routine looks like this (eax could be any general-purpose register): + * + * 8B 04 24 | mov eax, dword ptr [esp] + * C3 | ret + */ + std::optional getRoutineReturningSP(const Instruction& callInst); + protected: uint64_t m_fnAddress; uint64_t m_fnCallback; diff --git a/polyhook2/Detour/ILCallback.hpp b/polyhook2/Detour/ILCallback.hpp index e9630e5..103b227 100644 --- a/polyhook2/Detour/ILCallback.hpp +++ b/polyhook2/Detour/ILCallback.hpp @@ -1,76 +1,76 @@ -#ifndef POLYHOOK_2_0_ILCALLBACK_HPP -#define POLYHOOK_2_0_ILCALLBACK_HPP - -#pragma warning(push, 0) -#include -#pragma warning( pop ) - -#pragma warning( disable : 4200) -#include "polyhook2/PolyHookOs.hpp" -#include "polyhook2/ErrorLog.hpp" -#include "polyhook2/Enums.hpp" -#include "polyhook2/MemAccessor.hpp" - -namespace PLH { - class ILCallback : public MemAccessor { - public: - struct Parameters { - template - void setArg(const uint8_t idx, const T val) const { - *(T*)getArgPtr(idx) = val; - } - - template - T getArg(const uint8_t idx) const { - return *(T*)getArgPtr(idx); - } - - // asm depends on this specific type - // we the ILCallback allocates stack space that is set to point here - volatile uint64_t m_arguments; - private: - // must be char* for aliasing rules to work when reading back out - char* getArgPtr(const uint8_t idx) const { - return ((char*)&m_arguments) + sizeof(uint64_t) * idx; - } - }; - - struct ReturnValue { - unsigned char* getRetPtr() const { - return (unsigned char*)&m_retVal; - } - uint64_t m_retVal; - }; - - typedef void(*tUserCallback)(const Parameters* params, const uint8_t count, const ReturnValue* ret); - - ILCallback(); - ~ILCallback(); - - /* Construct a callback given the raw signature at runtime. 'Callback' param is the C stub to transfer to, - where parameters can be modified through a structure which is written back to the parameter slots depending - on calling convention.*/ - uint64_t getJitFunc(const asmjit::FuncSignature& sig, const asmjit::Arch arch, const tUserCallback callback); - - /* Construct a callback given the typedef as a string. Types are any valid C/C++ data type (basic types), and pointers to - anything are just a uintptr_t. Calling convention is defaulted to whatever is typical for the compiler you use, you can override with - stdcall, fastcall, or cdecl (cdecl is default on x86). On x64 those map to the same thing.*/ - uint64_t getJitFunc(const std::string& retType, const std::vector& paramTypes, const asmjit::Arch arch, const tUserCallback callback, std::string callConv = ""); - uint64_t* getTrampolineHolder(); - private: - // does a given type fit in a general purpose register (i.e. is it integer type) - bool isGeneralReg(const asmjit::TypeId typeId) const; - // float, double, simd128 - bool isXmmReg(const asmjit::TypeId typeId) const; - - asmjit::CallConvId getCallConv(const std::string& conv); - asmjit::TypeId getTypeId(const std::string& type); - - uint64_t m_callbackBuf; - asmjit::x86::Mem argsStack; - - // ptr to trampoline allocated by hook, we hold this so user doesn't need to. - uint64_t m_trampolinePtr; - }; -} -#endif // POLYHOOK_2_0_ILCALLBACK_HPP +#ifndef POLYHOOK_2_0_ILCALLBACK_HPP +#define POLYHOOK_2_0_ILCALLBACK_HPP + +#pragma warning(push, 0) +#include +#pragma warning( pop ) + +#pragma warning( disable : 4200) +#include "polyhook2/PolyHookOs.hpp" +#include "polyhook2/ErrorLog.hpp" +#include "polyhook2/Enums.hpp" +#include "polyhook2/MemAccessor.hpp" + +namespace PLH { + class ILCallback : public MemAccessor { + public: + struct Parameters { + template + void setArg(const uint8_t idx, const T val) const { + *(T*)getArgPtr(idx) = val; + } + + template + T getArg(const uint8_t idx) const { + return *(T*)getArgPtr(idx); + } + + // asm depends on this specific type + // we the ILCallback allocates stack space that is set to point here + volatile uint64_t m_arguments; + private: + // must be char* for aliasing rules to work when reading back out + char* getArgPtr(const uint8_t idx) const { + return ((char*)&m_arguments) + sizeof(uint64_t) * idx; + } + }; + + struct ReturnValue { + unsigned char* getRetPtr() const { + return (unsigned char*)&m_retVal; + } + uint64_t m_retVal; + }; + + typedef void(*tUserCallback)(const Parameters* params, const uint8_t count, const ReturnValue* ret); + + ILCallback(); + ~ILCallback(); + + /* Construct a callback given the raw signature at runtime. 'Callback' param is the C stub to transfer to, + where parameters can be modified through a structure which is written back to the parameter slots depending + on calling convention.*/ + uint64_t getJitFunc(const asmjit::FuncSignature& sig, const asmjit::Arch arch, const tUserCallback callback); + + /* Construct a callback given the typedef as a string. Types are any valid C/C++ data type (basic types), and pointers to + anything are just a uintptr_t. Calling convention is defaulted to whatever is typical for the compiler you use, you can override with + stdcall, fastcall, or cdecl (cdecl is default on x86). On x64 those map to the same thing.*/ + uint64_t getJitFunc(const std::string& retType, const std::vector& paramTypes, const asmjit::Arch arch, const tUserCallback callback, std::string callConv = ""); + uint64_t* getTrampolineHolder(); + private: + // does a given type fit in a general purpose register (i.e. is it integer type) + bool isGeneralReg(const asmjit::TypeId typeId) const; + // float, double, simd128 + bool isXmmReg(const asmjit::TypeId typeId) const; + + asmjit::CallConvId getCallConv(const std::string& conv); + asmjit::TypeId getTypeId(const std::string& type); + + uint64_t m_callbackBuf; + asmjit::x86::Mem argsStack; + + // ptr to trampoline allocated by hook, we hold this so user doesn't need to. + uint64_t m_trampolinePtr; + }; +} +#endif // POLYHOOK_2_0_ILCALLBACK_HPP diff --git a/polyhook2/Detour/x64Detour.hpp b/polyhook2/Detour/x64Detour.hpp index 0a45eb6..878c401 100644 --- a/polyhook2/Detour/x64Detour.hpp +++ b/polyhook2/Detour/x64Detour.hpp @@ -9,10 +9,8 @@ #include "polyhook2/Detour/ADetour.hpp" #include "polyhook2/Enums.hpp" #include "polyhook2/Instruction.hpp" -#include "polyhook2/ZydisDisassembler.hpp" -#include "polyhook2/ErrorLog.hpp" #include "polyhook2/RangeAllocator.hpp" -#include +#include namespace PLH { diff --git a/polyhook2/Detour/x86Detour.hpp b/polyhook2/Detour/x86Detour.hpp index f5cf2f3..1fd9bcf 100644 --- a/polyhook2/Detour/x86Detour.hpp +++ b/polyhook2/Detour/x86Detour.hpp @@ -7,9 +7,6 @@ #include "polyhook2/Detour/ADetour.hpp" #include "polyhook2/Enums.hpp" #include "polyhook2/Instruction.hpp" -#include "polyhook2/ZydisDisassembler.hpp" -#include "polyhook2/ErrorLog.hpp" -#include "polyhook2/MemProtector.hpp" using namespace std::placeholders; @@ -26,6 +23,8 @@ class x86Detour : public Detour { Mode getArchType() const override; protected: + void fixSpecialCases(insts_t& prologue); + bool makeTrampoline(insts_t& prologue, insts_t& trampolineOut); }; diff --git a/polyhook2/Instruction.hpp b/polyhook2/Instruction.hpp index 29bc05b..8278d0e 100644 --- a/polyhook2/Instruction.hpp +++ b/polyhook2/Instruction.hpp @@ -137,6 +137,10 @@ class Instruction { return m_isRelative; } + bool isReadingSP() const { + return m_isReadingSP; + } + /**Check if the instruction is a type with valid displacement**/ bool hasDisplacement() const { return m_hasDisplacement; @@ -178,10 +182,15 @@ class Instruction { return m_mnemonic + " " + m_opStr; } - /** Displacement size in bytes **/ - void setDisplacementSize(uint8_t size){ - m_dispSize = size; - } + /**Get parameters**/ + const std::string& getOperands() const { + return m_opStr; + } + + /** Displacement size in bytes **/ + void setDisplacementSize(uint8_t size) { + m_dispSize = size; + } size_t getDispSize() const { return m_dispSize; @@ -221,6 +230,10 @@ class Instruction { std::memcpy(&m_bytes[getDisplacementOffset()], &m_displacement.Absolute, dispSz); } + void setReadingSP(const bool isReadingSP) { + m_isReadingSP = isReadingSP; + } + long getUID() const { return m_uid.val; } @@ -326,6 +339,7 @@ class Instruction { bool m_isCalling; // Does this instruction is of a CALL type. bool m_isBranching; // Does this instruction jmp/call or otherwise change control flow bool m_isRelative; // Does the displacement need to be added to the address to retrieve where it points too? + bool m_isReadingSP; // Does this instruction read *SP in its second operand? bool m_hasDisplacement; // Does this instruction have the displacement fields filled (only rip/eip relative types are filled) bool m_hasImmediate; // Does this instruction have the immediate field filled? Displacement m_displacement; // Where an instruction points too (valid for jmp + call types, and RIP relative MEM types) @@ -337,7 +351,8 @@ class Instruction { uint8_t m_dispSize; // Size of the displacement, in bytes std::vector m_bytes; // All the raw bytes of this instruction - std::vector m_operands; // Types of all instruction operands + std::vector + m_operands; // Types of all instruction operands std::string m_mnemonic; std::string m_opStr; diff --git a/sources/ADetour.cpp b/sources/ADetour.cpp index 479392b..7ac5053 100644 --- a/sources/ADetour.cpp +++ b/sources/ADetour.cpp @@ -67,23 +67,30 @@ bool Detour::followJmp(insts_t& functionInsts, const uint8_t curDepth) { // NOLI return true; } - if (!m_isFollowCallOnFnAddress) - { + if (!m_isFollowCallOnFnAddress) { Log::log("setting: Do NOT follow CALL on fnAddress", ErrorLevel::INFO); - if (functionInsts.front().isCalling()) - { + if (functionInsts.front().isCalling()) { Log::log("First assembly instruction is CALL", ErrorLevel::INFO); return true; } } + const auto& firstInst = functionInsts.front(); + + // Here we know that first instruction is call imm32. + // But we first need to check if it is a special case #215. If it is, then we ignore it. + if (getRoutineReturningSP(firstInst)) { + Log::log("First assembly instruction is CALL, but for to a routine reading SP. Skipping.", ErrorLevel::INFO); + return true; + } + // might be a mem type like jmp rax, not supported - if (!functionInsts.front().hasDisplacement()) { + if (!firstInst.hasDisplacement()) { Log::log("Branching instruction without displacement encountered", ErrorLevel::WARN); return false; } - uint64_t dest = functionInsts.front().getDestination(); + const uint64_t dest = firstInst.getDestination(); functionInsts = m_disasm.disassemble(dest, dest, dest + 100, *this); return followJmp(functionInsts, curDepth + 1); // recurse } @@ -267,4 +274,23 @@ insts_t Detour::make_nops(uint64_t address, uint16_t size) const { return nops; } +std::optional Detour::getRoutineReturningSP(const Instruction& callInst) { + if (callInst.getMnemonic() != "call") { + return std::nullopt; + } + + const uint64_t routineAddress = callInst.getRelativeDestination(); + const insts_t routine = m_disasm.disassemble(routineAddress, routineAddress, routineAddress + 4, *this); + + if ( + routine.size() == 2 && + routine[0].getMnemonic() == "mov" && routine[0].hasRegister() && routine[0].isReadingSP() && + routine[1].getMnemonic() == "ret" + ) { + return routine; + } + + return std::nullopt; +} + } diff --git a/sources/ZydisDisassembler.cpp b/sources/ZydisDisassembler.cpp index 234519d..a0db668 100644 --- a/sources/ZydisDisassembler.cpp +++ b/sources/ZydisDisassembler.cpp @@ -40,7 +40,7 @@ PLH::insts_t PLH::ZydisDisassembler::disassemble( insts_t insVec; // m_branchMap.clear(); - uint64_t size = end - start; + int64_t size = end - start; assert(size > 0); if (size <= 0) { return insVec; @@ -139,7 +139,13 @@ void PLH::ZydisDisassembler::setDisplacementFields(PLH::Instruction& inst, const } case ZYDIS_OPERAND_TYPE_UNUSED: break; - case ZYDIS_OPERAND_TYPE_MEMORY: { // Relative to RIP/EIP + case ZYDIS_OPERAND_TYPE_MEMORY: { + if (i == 1 && operand->mem.base == ZYDIS_REGISTER_ESP) { + inst.setReadingSP(true); + break; + } + + // Relative to RIP/EIP inst.addOperandType(Instruction::OperandType::Displacement); if (zydisInst->attributes & ZYDIS_ATTRIB_IS_RELATIVE) { diff --git a/sources/x86Detour.cpp b/sources/x86Detour.cpp index a9f6f69..5daeb2b 100644 --- a/sources/x86Detour.cpp +++ b/sources/x86Detour.cpp @@ -1,12 +1,18 @@ // // Created by steve on 7/5/17. // +#include + +#include +#include + #include "polyhook2/Detour/x86Detour.hpp" namespace PLH { x86Detour::x86Detour(const uint64_t fnAddress, const uint64_t fnCallback, uint64_t* userTrampVar) - : Detour(fnAddress, fnCallback, userTrampVar, getArchType()) {} + : Detour(fnAddress, fnCallback, userTrampVar, getArchType()) { +} Mode x86Detour::getArchType() const { return Mode::x86; @@ -16,12 +22,61 @@ uint8_t getJmpSize() { return 5; } +/** + * @param prologue Must be before any relocations/translations were made + */ +void x86Detour::fixSpecialCases(insts_t& prologue) { + for (auto& instruction: prologue) { + if (const auto routine = getRoutineReturningSP(instruction)) { + Log::log( + "Fixing special case [call to routine returning ESP]:\n" + instsToStr(std::vector{instruction}), + ErrorLevel::INFO + ); + + // Fix for https://github.com/stevemk14ebr/PolyHook_2_0/issues/215 + // Example routine(eax could be any register): + // 8B 04 24 | mov eax, dword ptr [esp] + // C3 | ret + + const auto destReg = routine->at(0).getOperands().substr(0, 3); + const uint32_t originalAddress = instruction.getAddress(); + const uint32_t originalNextAddress = originalAddress + instruction.size(); + + // AsmTK parses strings for AsmJit, which generates the binary code. + asmjit::CodeHolder code; + asmjit::JitRuntime asmjitRt; + code.init(asmjitRt.environment()); + + asmjit::x86::Assembler assembler(&code); + asmtk::AsmParser parser(&assembler); + + // Parse the instructions via AsmTK + if (const auto error = parser.parse(std::format("mov {}, {:#x}", destReg, originalNextAddress).c_str())) { + Log::log(std::format("AsmTK error: {}", asmjit::DebugUtils::errorAsString(error)), ErrorLevel::SEV); + continue; + } + + // Generate the binary code via AsmJit + uint64_t movAddress = 0; + if (const auto error = asmjitRt.add(&movAddress, &code)) { + Log::log(std::format("AsmJIT error: {}", asmjit::DebugUtils::errorAsString(error)), ErrorLevel::SEV); + continue; + } + + // Replace `call rel32` instruction with `mov reg, imm32` + + instruction = m_disasm.disassemble(movAddress, movAddress, movAddress + instruction.size(), *this)[0]; + instruction.setAddress(originalAddress); + } + } +} + bool x86Detour::hook() { Log::log("m_fnAddress: " + int_to_hex(m_fnAddress) + "\n", ErrorLevel::INFO); - + insts_t insts = m_disasm.disassemble(m_fnAddress, m_fnAddress, m_fnAddress + 100, *this); Log::log("Original function:\n" + instsToStr(insts) + "\n", ErrorLevel::INFO); - + if (insts.empty()) { Log::log("Disassembler unable to decode any valid instructions", ErrorLevel::SEV); return false; @@ -92,6 +147,9 @@ bool x86Detour::hook() { bool x86Detour::makeTrampoline(insts_t& prologue, insts_t& trampolineOut) { assert(!prologue.empty()); + + fixSpecialCases(prologue); + const uint64_t prolStart = prologue.front().getAddress(); const uint16_t prolSz = calcInstsSz(prologue); @@ -99,7 +157,7 @@ bool x86Detour::makeTrampoline(insts_t& prologue, insts_t& trampolineOut) { address will change each attempt, which changes delta, which changes the number of needed entries. So we just try until we hit that lucky number that works. - The relocation could also because of data operations too. But that's specific to the function and can't + The relocation could also fail because of data operations too. But that's specific to the function and can't work again on a retry (same function, duh). Return immediately in that case. **/ uint8_t neededEntryCount = 5;