Incorrect x64 detour trampoline

[acidicoala]

Hello. When trying to hook a function in an x64 DLL, an incorrect trampoline seems to be generated. More specifically, a call instruction in function prologue gets messed up.

Log

[Polyhook] Original function:
7ff826849240 [4]: 48 83 ec 28                   sub rsp, 0x28
7ff826849244 [5]: e8 96 a8 ff ff                call 0x7ff826843adf -> 7ff826843adf
7ff826849249 [4]: 48 83 c4 28                   add rsp, 0x28
7ff82684924d [1]: cc                            int3 

[Polyhook] Prologue to overwrite:
7ff826849240 [4]: 48 83 ec 28                   sub rsp, 0x28
7ff826849244 [5]: e8 96 a8 ff ff                call 0x7ff826843adf -> 7ff826843adf

[Polyhook] Jmp To Prol:
1ee3c71e5b9 [6]: ff 25 49 00 00 00             jmp [1ee3c71e608] ->7ff826849249
1ee3c71e608 [8]: 49 92 84 26 f8 7f 00 00       dest holder 

[Polyhook] Trampoline:
1ee3c71e5b0 [4]: 48 83 ec 28                   sub rsp, 0x28
1ee3c71e5b4 [5]: e8 47 00 00 00                call 0x1ee3c71e600 -> 1ee3c71e600
1ee3c71e5b9 [6]: ff 25 49 00 00 00             jmp qword ptr [rip + 0x49] -> 7ff826849249

[Polyhook] Trampoline Jmp Tbl:
1ee3c71e600 [8]: df 3a 84 26 f8 7f 00 00       dest holder

---

The function to be hooked begins with a jump:

After hook is applied, this part remains intact as expected:

---

Notice that the jump destination has a call instruction in the 2nd position:

After hook is applied, the prologue gets replaced with a jump to the hooked function as expected:

---

Now, the trouble begins in the trampoline code. The operand to the call instruction does not point to a valid instruction address. Rather, it points to a memory location which stores the correct instruction address (it is included in the bottom part of the image). The generated trampoline however does not dereference it. This also results in call instruction being 1 byte shorter. Needless to say, it results in a crash:

Luckily, I could manually fix the call instruction by dereferencing its operand as follows:

With this fix, the program execution resumes without issues.

Since I was able to fix this error manually, I think it should be fixable on PolyHook side as well. But I'm not sure I can implement a polyhook fix myself, as my experience is limited in this area.

[stevemk14ebr]

Interesting, can you share either the binary or a hex dump of the function you are hooking? I will fix this, this is a big in the jump table logic. It seems to assume branching instructions in the prologue are jump - like, which obviously fails here.

[acidicoala]

Sure, here you go: uplay_r1_loader64.zip. The troublesome function is UPLAY_INSTALLER_GetLanguageUtf8.

[stevemk14ebr]

I think this is the bug here: https://github.com/stevemk14ebr/PolyHook_2_0/blob/master/sources/x64Detour.cpp#L424

I made a patch a while ago to correctly handle indirect calls of the style FF 25, which just need their displacement updated instead of the jump table actually holding a jump instruction. Seems I failed to handle the non-indirect call type correctly, so the call just points to an address instead of a jump table entry as it should. Need to think about how to fix this generally and if I missed any other issues but this is an easy fix.

FWIW, I never ever change the type of instructions, such as the e8 to ff 25 like you did. This works in simple cases like here but it gets really messy with more complex situtations since instruction sizes change. To avoid this, the concept of a jump table was introduced, where instructions are just re-written to point to little jump stubs that redirect the control flow. Since I control the jump table regions size, I don't have to care about instruction sizes in the prologue. The fix will look like this when it's done:

trampoline

sub rsp, 28h
call jmp_table_entry1
jmp qword ptr[jmp_table_entry2]

jmp_table:
entry1: jmp original_callsite
entry2: &original_prologue_after_overwrite

[stevemk14ebr]

Please try this patch out: ce93f6f. EDIT: use master, actively fixing bugs here.

[+] Info: Original function:
7ff7e3165048 [4]: 48 83 ec 28                   sub rsp, 0x28
7ff7e316504c [5]: e8 96 a8 ff ff                call 0x7ff7e315f8e7 -> 7ff7e315f8e7
7ff7e3165051 [4]: 48 83 c4 28                   add rsp, 0x28
7ff7e3165055 [7]: 48 ff a0 20 01 00 00          jmp qword ptr [rax + 0x120]


[+] Info: Prologue to overwrite:
7ff7e3165048 [4]: 48 83 ec 28                   sub rsp, 0x28
7ff7e316504c [5]: e8 96 a8 ff ff                call 0x7ff7e315f8e7 -> 7ff7e315f8e7


[+] Info: Jmp To Prol:
222f3302819 [6]: ff 25 49 00 00 00             jmp [222f3302868] ->7ff7e3165051
222f3302868 [8]: 51 50 16 e3 f7 7f 00 00       dest holder


[+] Info: Trampoline:
222f3302810 [4]: 48 83 ec 28                   sub rsp, 0x28
222f3302814 [5]: e8 06 00 00 00                call 0x222f330281f -> 222f330281f
222f3302819 [6]: ff 25 49 00 00 00             jmp qword ptr [rip + 0x49] -> 7ff7e3165051


[+] Info: Trampoline Jmp Tbl:
222f330281f [6]: ff 25 3b 00 00 00             jmp [222f3302860] ->7ff7e315f8e7
222f3302860 [8]: e7 f8 15 e3 f7 7f 00 00       dest holder

This jump table logic has been modified by other contributors, and it's gotten confusing. I may have introduced a bug by fixing this one. If you notice weird stuff please report - I may have to re-write this jump table logic entirely so I understand it again.

[acidicoala]

Thanks a lot, I will be testing the fixes in various environments during the next day or two.

[acidicoala]

The latest commit indeed fixes the issue at hand, and doesn't seem to break other detour hooks in both x86 and x64 environments. It actually solves another weird issue I had with a certain DLL, which took about 30 seconds to detour for each function. It now hooks instantaneously, like the rest.

I can submit a PR to vcpkg if you are comfortable with the latest commit.

[stevemk14ebr]

sure, go for it if your tests have passed. Even if there's a new bug, it's better than the old situation.