Sentry Issue: BINARYNINJA-7M
EXCEPTION_ACCESS_VIOLATION_READ / 0x0: Fatal Error: EXCEPTION_ACCESS_VIOLATION_READ / 0x0
File "dbgengttdadapter.cpp", line 210, in BinaryNinjaDebugger::DbgEngTTDAdapter::Reset
File "dbgengadapter.cpp", line 744, in BinaryNinjaDebugger::DbgEngAdapter::EngineLoop
@bdash says:
The crashing code is at
|
// There seems to be an internal ref-counting issue in the DbgEng TTD engine, that the reference for the debug |
|
// client is not properly freed after the target has exited. To properly free the debug client instance, here we |
|
// are calling Release() a few more times to ensure the ref count goes down to 0. Luckily this would not cause |
|
// a UAF or crash. |
|
// This might be related to the weird behavior of not terminating the target when we call TerminateProcesses(), |
|
// (see comment in `DbgEngTTDAdapter::Quit()`). |
|
// The same issue is not observed when we do forward debugging using the regular DbgEng. Also, I cannot reproduce |
|
// the issue using my script https://github.com/xusheng6/dbgeng_test. |
|
for (size_t i = 0; i < 100; i++) |
|
m_debugClient->Release(); |
The crash is a null pointer dereference that occurs when m_debugClient is null and the code attempts to load the vtable to make a virtual method call.
DbgEngTTDAdapter::Reset contains code guarded by a null check of this->m_debugClient, but later unconditionally accesses this->m_debugClient without any null check. It is this later access that crashes. Presumably it should be similarly guarded by a null check.
Additionally, there appears to be a thread-safety issue here where the DbgEngAdapter superclass protects some state via m_engineLoopMutex, but the DbgEngTTDAdapter subclass accesses that same state without acquiring the mutex. It is possible that this lack of synchronization could lead to m_debugClient being nulled out concurrently while the function is running, but I don't see any evidence that points to this being what caused this specific crash.
Sentry Issue: BINARYNINJA-7M
@bdash says: