Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MidHook x86: ESP is offset by -8 #48

Closed
ThirteenAG opened this issue Dec 22, 2023 · 8 comments · Fixed by #49 or #50
Closed

MidHook x86: ESP is offset by -8 #48

ThirteenAG opened this issue Dec 22, 2023 · 8 comments · Fixed by #49 or #50

Comments

@ThirteenAG
Copy link

Hey, I'm trying to move some hooks that originally used injector's MakeInline, which works in a similar manner, to use midhook instead.

For demo purposes, the hook only replicates original code.

Original game code:
.text:00A200DC F3 0F 11 4C 24 60 movss dword ptr [esp+60h], xmm1

injector version:

void operator()(injector::reg_pack& regs) 
{
    static float f = 0.0f;
    _asm { movss f, xmm1 }
    *(float*)(regs.esp + 0x60) = f;
}
MakeNOP(0xA200DC, 6); // because injector does that
safetyhook::create_mid(0xA200DC,
	[](SafetyHookContext& regs)
	{
		*(float*)(regs.esp + 0x60) = regs.xmm1.f32[0];
	});

Crashes later in that function.
I've compared the esp value and determined that doing

safetyhook::create_mid(0xA200DC,
	[](SafetyHookContext& regs)
	{
		*(float*)(regs.esp + 0x60 + 8) = regs.xmm1.f32[0];
	});

works fine.
I am not sure if this is intended or not, so let me know.
v0.2.0 was used to compile.

@angelfor3v3r
Copy link
Collaborator

angelfor3v3r commented Dec 22, 2023

https://github.com/cursey/safetyhook/blob/v0.2.0/include/safetyhook/context.hpp#L21-L23

Maybe this new note has something to do with it. Though, you're not really modifying RSP directly but rather stuff around it, so I'm not sure if it applies.

There's also some information in #46 & #47 about it. But I'm sure you already poked around these.

@cursey
Copy link
Owner

cursey commented Dec 22, 2023

@ThirteenAG can you try #49 and see if it fixes this behavior?

@ThirteenAG
Copy link
Author

@ThirteenAG can you try #49 and see if it fixes this behavior?

Yes, works. On an unrelated note, but at the same location, I can't seem to get rid of asm:

static float f = 0.0f;
_asm { movss f, xmm1 } // f == -0.000668681168

image
That way the hook works as intended.

If I do

static float f = 0.0f;
f = regs.xmm1.f32[0];

Then it stops working, probably because of this:
image

Other hooks work fine, only this one behaves this way, so I'm not sure it's safetyhook's issue.

Surrounding code for more context:

.text:00A200B3 12C E8 38 26 00 00                                call    sub_A226F0
.text:00A200B8 104 F3 0F 10 44 24 2C                             movss   xmm0, [esp+100h+var_D4]
.text:00A200BE 104 0F 57 C9                                      xorps   xmm1, xmm1
.text:00A200C1 104 0F 2E C1                                      ucomiss xmm0, xmm1
.text:00A200C4 104 9F                                            lahf
.text:00A200C5 104 F6 C4 44                                      test    ah, 44h
.text:00A200C8 104 7B 12                                         jnp     short loc_A200DC
.text:00A200CA 104 F3 0F 10 4C 24 48                             movss   xmm1, [esp+100h+var_B8]
.text:00A200D0 104 F3 0F 5C 8E 08 03 00 00                       subss   xmm1, dword ptr [esi+308h]
.text:00A200D8 104 F3 0F 5E C8                                   divss   xmm1, xmm0
.text:00A200DC
.text:00A200DC                                   loc_A200DC:                             ; CODE XREF: CCamFollowPed::sub_A1F9D0(void)+6F8↑j
>>> .text:00A200DC 104 F3 0F 11 4C 24 60                             movss   dword ptr [esp+60h], xmm1
.text:00A200E2 104 85 DB                                         test    ebx, ebx
.text:00A200E4 104 74 1B                                         jz      short loc_A20101
.text:00A200E6 104 F6 86 8C 03 00 00 04                          test    byte ptr [esi+38Ch], 4
.text:00A200ED 104 74 12                                         jz      short loc_A20101
.text:00A200EF 104 8D 44 24 18                                   lea     eax, [esp+100h+var_E8]
.text:00A200F3 104 50                                            push    eax
.text:00A200F4 108 8D 44 24 2C                                   lea     eax, [esp+104h+var_D8]
.text:00A200F8 108 50                                            push    eax
.text:00A200F9 10C 53                                            push    ebx
.text:00A200FA 110 8B CE                                         mov     ecx, esi
.text:00A200FC 110 E8 4F DD FF FF                                call    sub_A1DE50

@cursey cursey reopened this Dec 23, 2023
@cursey
Copy link
Owner

cursey commented Dec 23, 2023

From the look of it, it seems like xmm1 has been captured incorrectly. Was it working before #49? Are you able to provide a minimal test that reproduces this issue?

@ThirteenAG
Copy link
Author

Was it working before #49? Are you able to provide a minimal test that reproduces this issue?

No, it wasn't. I'll prepare an example.

@ThirteenAG
Copy link
Author

ThirteenAG commented Dec 23, 2023

I think that should be it:

#include <iostream>
#include <safetyhook.hpp>

__declspec(noinline) void test() {
    std::cout << "test\n";
}

__declspec(noinline) int add_42(int a) {

    static float f = 0.111f;
    _asm { movss xmm1, f }
    _asm { test    ebx, ebx }

    _asm call dword ptr ds : [test]

    return a + 42;
}

void hooked_test(SafetyHookContext& regs) {

    static volatile float f1 = 0.0f;
    static volatile float f2 = 0.0f;
    _asm { movss f1, xmm1 }
    f2 = regs.xmm1.f32[0];


    regs.eax = 1337;
}

SafetyHookMid g_hook{};

int main() {
    g_hook = safetyhook::create_mid(&test, hooked_test);

    std::cout << add_42(3) << "\n";

    std::cout << add_42(4) << "\n";

    return 0;
}

image

@cursey cursey mentioned this issue Dec 23, 2023
@cursey
Copy link
Owner

cursey commented Dec 23, 2023

Thanks for the test @ThirteenAG . Can you try #50 and let me know if it fixes it for you?

@ThirteenAG
Copy link
Author

Thanks for the test @ThirteenAG . Can you try #50 and let me know if it fixes it for you?

Works like a charm, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants