From c57571f430cb30db3163d09ab4894e197edfdf47 Mon Sep 17 00:00:00 2001 From: matcool <26722564+matcool@users.noreply.github.com> Date: Mon, 3 Jun 2024 00:56:23 -0300 Subject: [PATCH] fix stack args on windows64 --- src/convention/Windows64Convention.cpp | 25 +++++++++++---- src/generator/X64Generator.cpp | 6 +++- test/CMakeLists.txt | 7 +++- test/Hook.cpp | 44 ++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/convention/Windows64Convention.cpp b/src/convention/Windows64Convention.cpp index 1515c9d..f306de1 100644 --- a/src/convention/Windows64Convention.cpp +++ b/src/convention/Windows64Convention.cpp @@ -24,17 +24,20 @@ namespace { } return stackParamSize; } + size_t getPaddedStackParamSize(AbstractFunction const& function) { + auto stackParamSize = getStackParamSize(function); + return (stackParamSize % 16) ? stackParamSize + 8 : stackParamSize; + } } ThiscallConvention::~ThiscallConvention() {} void ThiscallConvention::generateDefaultCleanup(BaseAssembler& a_, AbstractFunction const& function) { - size_t stackParamSize = getStackParamSize(function); - if (stackParamSize > 0) { + size_t paddedSize = getPaddedStackParamSize(function); + if (paddedSize > 0) { auto& a = static_cast(a_); using enum X64Register; - auto const paddedSize = (stackParamSize % 16) ? stackParamSize + 8 : stackParamSize; - a.add(RSP, paddedSize); + a.add(RSP, paddedSize + 0x20); } } @@ -60,12 +63,20 @@ void ThiscallConvention::generateIntoDefault(BaseAssembler& a_, AbstractFunction size_t stackParamSize = getStackParamSize(function); if (stackParamSize > 0) { auto const paddedSize = (stackParamSize % 16) ? stackParamSize + 8 : stackParamSize; - a.sub(RSP, paddedSize); + // + 0x20 for the shadow space before the first arg + a.sub(RSP, paddedSize + 0x20); int stackOffset = 0; + // RBP points to this (each cell is 8 bytes): + // [orig rbp] [return ptr] [] [] [] [] [1st stack arg] ... + // rbp + 0 rbp + 8 rbp + 0x30 ... + // new stack will look like + // [] [] [] [] [1st stack arg] ... + // rsp + 0x20 ... + for (auto i = 0; i < stackParamSize; i += 8) { - a.mov(RAX, m[RBP + (32 + i)]); - a.mov(m[RSP + i], RAX); + a.mov(RAX, m[RBP + (0x30 + i)]); + a.mov(m[RSP + (0x20 + i)], RAX); } } } diff --git a/src/generator/X64Generator.cpp b/src/generator/X64Generator.cpp index fe94d5e..21e2983 100644 --- a/src/generator/X64Generator.cpp +++ b/src/generator/X64Generator.cpp @@ -155,15 +155,19 @@ std::vector X64HandlerGenerator::handlerBytes(uint64_t address) { // call the pre handler, incrementing a.callip("handlerPre"); + // store rax (next function ptr) in the shadow space for a bit a.mov(m[RBP - 0x10], RAX); // recover registers restoreRegisters(a, preservedSize); - // call the func + // convert the current cc into the default cc m_metadata.m_convention->generateIntoDefault(a, m_metadata.m_abstract); + // restore the next function ptr from shadow space a.mov(RAX, m[RBP - 0x10]); + + // call the func // a.int3(); a.call(RAX); // // a.int3(); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index eb05762..5a9d0cd 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -12,7 +12,12 @@ add_executable(${PROJECT_NAME} Assembler86.cpp Assembler64.cpp Hook.cpp) target_link_libraries(${PROJECT_NAME} PUBLIC GTest::gtest_main TulipHook) if(WIN32) - set_target_properties(${PROJECT_NAME} PROPERTIES LINK_FLAGS "/INCREMENTAL:NO") + if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND + CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "GNU") + # idk + else() + set_target_properties(${PROJECT_NAME} PROPERTIES LINK_FLAGS "/INCREMENTAL:NO") + endif() if (CMAKE_SIZEOF_VOID_P EQUAL 8) add_executable(WinTest misc/WinTest.cpp) diff --git a/test/Hook.cpp b/test/Hook.cpp index f32b12b..0429997 100644 --- a/test/Hook.cpp +++ b/test/Hook.cpp @@ -194,3 +194,47 @@ TEST(HookTest, RecreateHandler) { EXPECT_EQ(callFunction<10>(), 1); } + +int checkParams(int a, int b, int c, int d, int e, int f, int g, float h, int i) { + EXPECT_EQ(a, 1); + EXPECT_EQ(b, 2); + EXPECT_EQ(c, 3); + EXPECT_EQ(d, 4); + EXPECT_EQ(e, 5); + EXPECT_EQ(f, 6); + EXPECT_EQ(g, 7); + EXPECT_EQ(h, 8.0f); + EXPECT_EQ(i, 9); + return 11; +} + +int checkParamsHook(int a, int b, int c, int d, int e, int f, int g, float h, int i) { + EXPECT_EQ(a, 1); + EXPECT_EQ(b, 2); + EXPECT_EQ(c, 3); + EXPECT_EQ(d, 4); + EXPECT_EQ(e, 5); + EXPECT_EQ(f, 6); + EXPECT_EQ(g, 7); + EXPECT_EQ(h, 8.0f); + EXPECT_EQ(i, 9); + return checkParams(a, b, c, d, e, f, g, h, i); +} + +TEST(HookTest, SingleHookCheckParams) { + HandlerMetadata handlerMetadata; + // cheating using thiscall because default convention is not properly implemented + handlerMetadata.m_convention = tulip::hook::createConvention(tulip::hook::TulipConvention::Thiscall); + handlerMetadata.m_abstract = AbstractFunction::from(&checkParams); + + auto handleRes = createHandler(reinterpret_cast(&checkParams), handlerMetadata); + + ASSERT_FALSE(handleRes.isErr()) << "Failed to create handler: " << handleRes.unwrapErr(); + + auto handle = handleRes.unwrap(); + + HookMetadata metadata; + createHook(handle, reinterpret_cast(&checkParamsHook), metadata); + + checkParams(1, 2, 3, 4, 5, 6, 7, 8.0f, 9); +} \ No newline at end of file