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

Improper hooks when RSP instructions are in the prologue #187

Open
BritishPiper opened this issue Jul 29, 2023 · 1 comment
Open

Improper hooks when RSP instructions are in the prologue #187

BritishPiper opened this issue Jul 29, 2023 · 1 comment

Comments

@BritishPiper
Copy link
Contributor

BritishPiper commented Jul 29, 2023

Polyhook fails to properly translate prologues that directly modify rsp (e.g. mov rsp, [0xDEADBEEF]).
This is a very uncommon thing, but I've encountered a DRM using this for whatever reason.
The current translation by x64Detour::generateTranslationRoutine is:

lea rsp, rsp-0x80
push rax
push r15
mov r15, orig_rip + 0xDEADBEEF
mov rax, [r15]
mov rsp, rax
pop r15
pop rax
push rax
mov rax, return_addr
xchg [rsp], rax
ret 80

I don't know if this is an actual problem you'd want to solve, but a solution would be to not push/pop and instead use another scratch register as your stack pointer (usually rbp).
Then restore it later with something like "mov rbp, [rbp - where_you_saved_rbp_in_the_stack]".

Also, shadow space spoiling prevention would need to go (lea rsp, [rsp - 0x80] and ret 80), but I don't see a need for it if you choose to never mess with rsp in the first place.


On a side note, a minor thing I couldn't understand is why use r15 when you could just mov rax, [rax]. There are definitely situations where that scratch register is needed, but not for simple cases (like cmp [0xDEADBEEF], 0). Probably not important though.

@stevemk14ebr
Copy link
Owner

You are right I could just use RAX, I will probably switch to that.

Can you explain what you mean by lea rsp, [rsp-0x80]? Is that a mistake in the current implementation or are you saying it's a modification needed for your suggestion.

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

No branches or pull requests

2 participants