search

DynamoRIO / dynamorio

Dynamic Instrumentation Tool Platform
Other
2.67k stars 562 forks source link

load_dynamo() can silently fail during child / code injection on Windows #4282

Open gaasedelen opened 4 years ago

gaasedelen commented 4 years ago

Describe the bug

During the code injection / dynamorio bringup within the target process, LoadLibrary("dynamorio.dll") can fail for any number of reasons. Worse is that there is no error logged and no notification to the user of this failure and drrun.exe waits for the process to exit as if everything is nominal.

To Reproduce

Explicitly reproducing this might be difficult (ASLR randomness), but I have attached a TTD trace that can be used in WinDBG Preview to observe the execution of drrun.exe and the target process ida64.exe in an instance of this failure.

TTD Traces: https://drive.google.com/open?id=1URuyKYhMzR9QcBbYXVlnL9OB25cfGYnW

The code for the stub is found here. To debug the load_dynamo() stub in the child trace, open ida6401.run with WinDBG Preview and run the following commands:

bp 0x0000026fe7df0000
g

The LoadLibrary call that fails is a few instructions later, at 0000026fe7df0015. This is because the static imagebase for dynamorio.dll conflicted with libraries already loaded by the process.

(dynamorio.dll should be rebasable imo, but that's an entirely separate issue...)

Oh, and the command run in the VM was: 

"C:\Users\User\Desktop\DynamoRIO-Windows-8.0.0-1\bin64\drrun.exe" -t drcov -- "C:\Program Files\IDA Freeware 7.0\ida64.exe"

Expected behavior

If dynamorio cannot successfully initialize within the child process, there should be a message printed from drrun.exe stating such failure and the child should exit.

Alternatively, the injected stub should pop a MessageBox or something to notify of such failure.

In my opinion, it makes no sense to let the child process continue running if dynamorio cannot be injected. Why would I want it to continue running if the instrumentation can't be loaded?!? It's both confusing and less-indicative that something has gone very wrong.

Versions

Additional context

The enterprise VM has some of the 'exploit guard' mitigations on, like high-entropy ASLR. These could be aggravating the issue but i'm not sure.

Special thanks to @toshipiazza for sweating it out with me while we spitballed ideas on the erratic failures. We burned a solid 6-8 hours tracking this issue down 😬

derekbruening commented 4 years ago

Re: error handling: The current injection error handling was geared toward an older use case where DR injected into every Windows service at boot time, where killing the child could blue-screen the machine and was not acceptable: instead DR would let the app continue natively (writing an error message to the event log if possible). For today's typical use of targeted injection of one particular regular non-system-service app, yes, a different error handling is called for.

This is because the static imagebase for dynamorio.dll conflicted with libraries already loaded by the process. (dynamorio.dll should be rebasable imo, but that's an entirely separate issue...)

It should be rebasable. This sounds like a regression. Maybe you could file a separate issue (and ideally figure out why it's not...some recent asm code missing relocations? and add a regression test to ensure) if this issue is about the error handling.

Ideally this old, late injection method would be deprecated and removed and replaced by injection at an earlier point with library mapping from the parent (-early inject, various flavors; some inject gencode to load but the cleanest does the mapping from the parent), but supporting clients using system libraries makes that very hard. For just drcov, it should be doable, but probably there's bitrot and minor issues to work out to turn those on by default.