-
Notifications
You must be signed in to change notification settings - Fork 409
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
The function DetourCreateRemoteNative32 is thread safety? #75
Comments
windbg output information:
|
First of all, no, There are two problems here:
(1) cannot truly be fixed as far as I can see. The problem can be mitigated somewhat by suspending the process before applying the hooks and then resuming it (this reduces the number of AV-generating states the application can be in while SH is changing memory properties), but if a thread happens to get suspended in the middle of executing a syscall stub, it will still crash the process when it is resumed. I guess technically it would be possible to detect such threads by checking their EIP/RIP with (2) can be fixed by doing proper unhooking of hooks already in place before reinjecting. This is also something that can only be done by a plugin, because the CLI injector doesn't know about any past injections by plugins or previous versions of itself. How much work this would be to fix, I'm not sure. I think this doesn't affect x64, only WOW64 and maybe native x86, since they both use a shared syscall gateway that may or may not be restored correctly. I will add some primitive hook detection to at least prevent a crash, but this also isn't a true fix and more of a bandaid. Note that situation (1) can occur when restoring hooks just like it can occur when applying them. TLDR: debugger plugins are preferable over the CLI injector, starting in a debugger is preferable to attaching, and avoid changing options/reapplying hooks at runtime if you can. The last one should still be OK on x64 processes though. |
i already compiled the latest commit, and it still crashes , the details about the bug reproduce video(pwd: 123): http://s000.tinyupload.com/index.php?file_id=09317845334746442175 i use |
I know the commit didn't fix the issue, otherwise I would have closed it. Read (2) again. |
... i notice this😂, i just use |
Wow, I'm impressed you managed that. I did reproduce this eventually without having to reinject (after trying for a long time...), but it is definitely hard to do. The reason I couldn't reproduce this was because I wasn't debugging ScyllaHide.
If above conditions are met, So this is a race condition that is very hard to pull off... unless you are stepping through ScyllaHide in Visual Studio like in your video, while the process is running and executing a I moved the suspend/resume region so that both the hooking and the writing of hook DLL data happen while the process is suspended. It now Works For Me (TM). Let me know if this fixes it for you - I'm guessing this is a reduced test case of a real app. Leaving this open for now because issue (2) from my first post is not fixed. |
this fixes is great for me 👍 |
because issue (2) is not fixed i haven't close issue immediately |
It turns out the combination of the first and second commits (detect already installed hooks, and suspend the process for the duration of the entire injection procedure) fixed issue (2) as well and there is no more crash on reinjection. The CLI still can't inject a second time since it never unhooks, but this is by design. Closing this. |
usage:
The console applications will be crashes when i testing.
i think is thread(eip) Execute here:
and at this time
ScyllaHide
useWriteProcessMemory
make it(ConsoleApplication1.exe)
execute cpu instruction errorScyllaHide/InjectorCLI/RemoteHook.cpp
Line 429 in e9def65
scylla_hide configuration file(png -> ini):
The text was updated successfully, but these errors were encountered: