Fix/#215

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);

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()
 	);
 

UnitTests/windows/TestDetourx64.cpp

@@ -9,7 +9,7 @@
 
 #include "polyhook2/PolyHookOsIncludes.hpp"
 
-#include <asmjit/asmjit.h>
+#include <asmjit/x86.h>
 
 #define WIN32_LEAN_AND_MEAN
 #include <Windows.h>

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<insts_t> getRoutineReturningSP(const Instruction& callInst);
+
 protected:
     uint64_t m_fnAddress;
     uint64_t m_fnCallback;

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 <asmjit/asmjit.h>
-#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<typename T>
-			void setArg(const uint8_t idx, const T val) const {
-				*(T*)getArgPtr(idx) = val;
-			}
-
-			template<typename T>
-			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<std::string>& 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 <asmjit/x86.h>
+#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<typename T>
+			void setArg(const uint8_t idx, const T val) const {
+				*(T*)getArgPtr(idx) = val;
+			}
+
+			template<typename T>
+			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<std::string>& 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

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 <asmjit/asmjit.h>
+#include <asmjit/x86.h>
 
 namespace PLH {
 

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);
 };
 

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<uint8_t> m_bytes;    // All the raw bytes of this instruction
-	std::vector<OperandType> m_operands; // Types of all instruction operands
+	std::vector<OperandType>
+	m_operands; // Types of all instruction operands
 	std::string          m_mnemonic;
 	std::string          m_opStr;
 

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,26 @@ insts_t Detour::make_nops(uint64_t address, uint16_t size) const {
     return nops;
 }
 
+std::optional<insts_t> 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) {
+        return std::nullopt;
+    }
+
+    if (
+        routine.size() == 2 &&
+        routine[0].getMnemonic() == "mov" && routine[0].hasRegister() && routine[0].isReadingSP() &&
+        routine[1].getMnemonic() == "ret"
+    ) {
+        return routine;
+    }
+
+    return std::nullopt;
+}
+
 }

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) {