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

Fix injector: drakvuf_set_vcpu_gprs forgot about RSI and RDI #1800

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

psrok1
Copy link
Contributor

@psrok1 psrok1 commented Aug 12, 2024

Hi!

This fix is addressing issues related with target process crash/hang caused by injector e.g. #925, #1758

I've spent hours on debugging injector crashes and I found a bit surprising bug: hijacked thread context is incorrectly restored because two registers were missing in drakvuf_set_vcpu_gprs function: RSI and RDI.

These registers are crucial because according to Microsoft x64 ABI convention, they're non-volatile registers that should be preserved by the callee. Of course the bug leads to the corruption of targeted process, usually followed by a crash.

I found the bug by debugging one of the common places where explorer.exe was spotted to crash after injection. In this case, injector traps on the return from ZwWaitForWorkViaWorkerFactory where hijacked thread wakes up. The clobbered RSI register is then used in various places as it points to some shared structure with PSRWLOCK pointer in rsi+0x48. Final result is usually an EXCEPTION_ACCESS_VIOLATION crash in RtlAcquireSRWLockExclusive.

Other register values are correctly restored excluding RSI and RDI:
image
image

RtlAcquireSRWLockExclusive argument evaluated from RSI:
image
The faulting instruction:
image

@drakvuf-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@tklengyel
Copy link
Owner

@drakvuf-jenkins Test this please

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 this pull request may close these issues.

4 participants