Zeex / subhook

Simple hooking library for C/C++ (x86 only, 32/64-bit, no dependencies)
BSD 2-Clause "Simplified" License
801 stars 124 forks source link

subhook_make_jmp64 might SIGSEGV if a function is located across a page boundary #59

Open cube0x8 opened 3 years ago

cube0x8 commented 3 years ago

Hello,

I am successfully using subhook in loadlibrary so, first of all, thank you for your effort to put this project together!

Recently I might have discovered a bug in subhook: I noticed that when mprotect is executed, even if size == sizeof(struct subhook_jmp64), the permission are changed on the entire page instead.

For example, let's say we are creating an x86_64 hook on the following function:

(gdb) x/1i src
   0x55de0714b9aa <DeleteCriticalSection>:  push   %rbp

After we executed mprotect (in subhook_unprotect()): return mprotect(address, size, SUBHOOK_CODE_PROTECT_FLAGS); (with size == 0x14)

the following output is returned by cat /proc/$PID/maps:

55de0714b000-55de0714c000 rwxp 00014000 08:01 787782

So, as you can notice, it looks like the permissions are changed on 0x1000 bytes instead of 0x14.

This said, we could face a scenario where the first 0x14 of a function are located across a page boundary, so not all of these 0x14 will become RWX.

For example, let's consider this function:

(gdb) x/1i address
   0x55de0714bff8 <CertStrToNameW>: push   %rbp

whose the page address range is 0x55de0714b000 - 0x55de0714c000 and the first 0x14 bytes of the function ends between 0x55de0714bff8 and 0x55de0714c00c. It's easy to spot how the privileges of the very first 0x8 bytes of the function are affected from the call to mprotect, while the other 0xc remains unchanged.

This will inevitably lead to a SIGSEGV in subhook_make_jmp64.

Here, I implemented a quick fix which works. Feel free to cherry-pick it, if you want.

I hope the explanation was exhausting and I made the entire point clear.

FilipeSilvens commented 3 years ago

I'm pretty sure this is an issue with mprotect Using the code you posted still shows the permissions shared over 0x1000 bytes image

cube0x8 commented 3 years ago

Sorry, but I don't get your point. Which code are you talking about?

I guess I was not clear enough in my first comment, so let me try to explain it better:

As you said, the permissions are shared over 0x1000 bytes which is right, but the problem arises when a function is located at page boundary.

Let's take your screenshot as example: e.g. we have a function which is located at 0x7faff2692fef. mprotect (which takes a page aligned address as argument, otherwise it fails) will change permission as RWX from 0x7faff2692000 to 0x7faff2693000 (as showed in the picture). But later on, subhook will try to patch the first 0x14 bytes of the function, which are located in memory from 0x7faff2692fef to 0x7faff2693003. Conclusion: we have exactly 0x3 bytes which are not subjected to the mprotect changes, which eventually will lead to a SIGSEGV.

Spiros-N-Agathos commented 3 years ago

I also came across this issue today. In Swarm64 (https://swarm64.com/) we are using the subhook library, and got seg fault because of a page boundary issue. Applying the fix that @cube0x8 (big acknowledgements btw) proposed solved the problem.