diff --git a/core/adapters/dbgengadapter.cpp b/core/adapters/dbgengadapter.cpp index 705bed7..b66a057 100644 --- a/core/adapters/dbgengadapter.cpp +++ b/core/adapters/dbgengadapter.cpp @@ -73,9 +73,17 @@ std::string DbgEngAdapter::GetDbgEngPath(const std::string& arch) else path = Settings::Instance()->Get("debugger.x86dbgEngPath"); - if (IsValidDbgEngPaths(path)) - return path; + if (!path.empty()) + { + // If the user has specified the path in the setting, then check it for validity. If it is valid, then use it; + // if it is invalid, fail the operation -- do not fallback to the default one + if (IsValidDbgEngPaths(path)) + return path; + else + return ""; + } + // If the user does not specify a path (the default case), find the one from the %APPDATA% or %PROGRAMDATA% char appData[MAX_PATH]; // For a system installation, the path is %APPDATA%\Binary Ninja\dbgeng\Windows Kits\10\Debuggers if (SUCCEEDED(SHGetFolderPathA(NULL, CSIDL_APPDATA, NULL, 0, appData))) @@ -97,54 +105,99 @@ std::string DbgEngAdapter::GetDbgEngPath(const std::string& arch) return ""; } -bool DbgEngAdapter::LoadDngEngLibraries() + +static bool LoadOneDLL(const string& path, const string& name, bool strictCheckPath = true, bool forceUnload = true) { - auto enginePath = GetDbgEngPath("x64"); - if (!enginePath.empty()) + auto handle = GetModuleHandleA(name.c_str()); + if (handle) { - LogDebug("DbgEng libraries in path %s", enginePath.c_str()); - if (!SetDllDirectoryA(enginePath.c_str())) - LogWarn( - "Failed to set DLL directory to %s. The debugger is going to load the system dbgeng DLLs, which may " - "not work as expected", - enginePath.c_str()); - } - else - { - LogWarn( - "debugger.x64dbgEngPath is empty or invalid. The debugger is going to load the system dbgeng DLLs, which " - "may " - "not work as expected"); + LogDebug("Module %s is already loaded before the debugger tries to load it, this is suspicious", name.c_str()); + if (!strictCheckPath) + // The module is already loaded and we do not wish to validate its path, treat it as a success + return true; + + char actualPath[MAX_PATH]; + if (!GetModuleFileNameA(handle, actualPath, MAX_PATH)) + { + LogWarn("Failed to get the path of the loaded %s, error: %lu", name.c_str(), GetLastError()); + return false; + } + string path1 = actualPath; + std::transform(path1.begin(), path1.end(), path1.begin(), ::toupper); + string path2 = path + '\\' + name; + std::transform(path2.begin(), path2.end(), path2.begin(), ::toupper); + if (path1 == path2) + // two paths match, ok + return true; + + LogWarn("%s is loaded from %s, but we expect it from %s", name.c_str(), actualPath, path.c_str()); + if (!forceUnload) + return false; + + size_t unloadMaxTries = 100; + bool unloaded = false; + for (size_t i = 0; i < unloadMaxTries; i++) + { + FreeLibrary(handle); + handle = GetModuleHandleA(name.c_str()); + if (handle == NULL) + { + unloaded = true; + break; + } + } + if (!unloaded) + { + LogDebug("Failed to unload module %s", name.c_str()); + return false; + } + else + { + LogDebug("Module %s has been unloaded", name.c_str()); + } } - HMODULE handle; - handle = LoadLibraryA("dbgcore.dll"); + auto dllFullPath = path + '\\' + name; + handle = LoadLibraryA(dllFullPath.c_str()); if (handle == nullptr) { - LogWarn("Failed to load dbgcore.dll, %d", GetLastError()); + LogWarn("Failed to load %s, error: %lu", dllFullPath.c_str(), GetLastError()); return false; } - handle = LoadLibraryA("dbghelp.dll"); - if (handle == nullptr) + return true; +} + + +bool DbgEngAdapter::LoadDngEngLibraries() +{ + auto enginePath = GetDbgEngPath("x64"); + if (enginePath.empty()) { - LogWarn("Failed to load dbghelp.dll, %d", GetLastError()); + LogWarn("The debugger cannot find the path for the DbgEng DLLs. " + "If you have set debugger.x64dbgEngPath, check its correctness; if you have not set it, " + "reinstall the DbgEng redistributable"); return false; } + LogDebug("DbgEng libraries in path %s", enginePath.c_str()); - handle = LoadLibraryA("dbgmodel.dll"); - if (handle == nullptr) - { - LogWarn("Failed to load dbgmodel.dll, %d", GetLastError()); + auto settings = Settings::Instance(); + auto strictCheckPath = settings->Get("debugger.checkDbgEngDLLPath"); + auto forceUnload = settings->Get("debugger.tryUnloadWrongDbgEngDLL"); + + if (!LoadOneDLL(enginePath, "dbghelp.dll", strictCheckPath, forceUnload)) return false; - } - handle = LoadLibraryA("dbgeng.dll"); - if (handle == nullptr) - { - LogWarn("Failed to load dbgeng.dll, %d", GetLastError()); + if (!LoadOneDLL(enginePath, "dbgcore.dll", strictCheckPath, forceUnload)) return false; - } + + if (!LoadOneDLL(enginePath, "dbgmodel.dll", strictCheckPath, forceUnload)) + return false; + + if (!LoadOneDLL(enginePath, "dbgeng.dll", strictCheckPath, forceUnload)) + return false; + + return true; } std::string DbgEngAdapter::GenerateRandomPipeName() @@ -237,7 +290,11 @@ bool DbgEngAdapter::Start() auto pipeName = GenerateRandomPipeName(); auto connectString = fmt::format("npipe:pipe={},Server=localhost", pipeName); auto arch = m_defaultArchitecture == "x86_64" ? "x64" : "x86"; - auto dbgsrvCommandLine = fmt::format("\"{}\\dbgsrv.exe\" -t {}", GetDbgEngPath(arch), connectString); + auto enginePath = GetDbgEngPath(arch); + if (enginePath.empty()) + return false; + + auto dbgsrvCommandLine = fmt::format("\"{}\\dbgsrv.exe\" -t {}", enginePath, connectString); if (!LaunchDbgSrv(dbgsrvCommandLine)) { LogWarn("Command %s failed", dbgsrvCommandLine.c_str()); @@ -324,7 +381,7 @@ DbgEngAdapter::~DbgEngAdapter() bool DbgEngAdapter::Init() { - return LoadDngEngLibraries(); + return true; } diff --git a/core/adapters/dbgengadapter.h b/core/adapters/dbgengadapter.h index 3b02392..e5cd56b 100644 --- a/core/adapters/dbgengadapter.h +++ b/core/adapters/dbgengadapter.h @@ -232,9 +232,9 @@ namespace BinaryNinjaDebugger { void ApplyBreakpoints(); - virtual std::string GetDbgEngPath(const std::string& arch); + static std::string GetDbgEngPath(const std::string& arch); - bool LoadDngEngLibraries(); + static bool LoadDngEngLibraries(); std::string GenerateRandomPipeName(); diff --git a/core/adapters/dbgengttdadapter.cpp b/core/adapters/dbgengttdadapter.cpp index cb19657..8a4f878 100644 --- a/core/adapters/dbgengttdadapter.cpp +++ b/core/adapters/dbgengttdadapter.cpp @@ -11,50 +11,6 @@ DbgEngTTDAdapter::DbgEngTTDAdapter(BinaryView* data) : DbgEngAdapter(data) } -static bool IsValidDbgEngPaths(const std::string& path) -{ - if (path.empty()) - return false; - - auto enginePath = filesystem::path(path); - if (!filesystem::exists(enginePath)) - return false; - - if (!filesystem::exists(enginePath / "dbgeng.dll")) - return false; - - if (!filesystem::exists(enginePath / "dbghelp.dll")) - return false; - - if (!filesystem::exists(enginePath / "dbgmodel.dll")) - return false; - - if (!filesystem::exists(enginePath / "dbgcore.dll")) - return false; - - if (!filesystem::exists(enginePath / "dbgsrv.exe")) - return false; - - if (!filesystem::exists(enginePath / "ttd")) - return false; - - if (!filesystem::exists(enginePath / "ttd" / "TTDReplay.dll")) - return false; - - return true; -} - - -std::string DbgEngTTDAdapter::GetDbgEngPath(const std::string& arch) -{ - std::string path = Settings::Instance()->Get("debugger.x64dbgEngPath"); - if (IsValidDbgEngPaths(path)) - return path; - - return ""; -} - - bool DbgEngTTDAdapter::ExecuteWithArgsInternal(const std::string& path, const std::string& args, const std::string& workingDir, const LaunchConfigurations& configs) { m_aboutToBeKilled = false; diff --git a/core/adapters/dbgengttdadapter.h b/core/adapters/dbgengttdadapter.h index 3f0c38d..8f8450f 100644 --- a/core/adapters/dbgengttdadapter.h +++ b/core/adapters/dbgengttdadapter.h @@ -32,8 +32,6 @@ namespace BinaryNinjaDebugger { void Reset() override; bool Quit() override; - - std::string GetDbgEngPath(const std::string& arch) override; }; class DbgEngTTDAdapterType : public DebugAdapterType diff --git a/core/adapters/lldbadapter.cpp b/core/adapters/lldbadapter.cpp index 9d1b504..484c8d4 100644 --- a/core/adapters/lldbadapter.cpp +++ b/core/adapters/lldbadapter.cpp @@ -74,14 +74,16 @@ DebugAdapter* LldbAdapterType::Create(BinaryNinja::BinaryView* data) // the liblldb.dll is located. // As a note, the reason for us to apply delay load on liblldb.dll is that if we load it early, it will also load // the system's default dbgeng dlls, which does not work for our dbgeng adapter. + std::string lldbDir; if (getenv("BN_STANDALONE_DEBUGGER") != nullptr) - SetDllDirectoryA(GetUserPluginDirectory().c_str()); + lldbDir = GetUserPluginDirectory(); else - SetDllDirectoryA(GetBundledPluginDirectory().c_str()); + lldbDir = GetBundledPluginDirectory(); - auto module = LoadLibraryA("liblldb.dll"); + auto lldbPath = lldbDir + '\\' + "liblldb.dll"; + auto module = LoadLibraryA(lldbPath.c_str()); if (module == NULL) - throw std::runtime_error("fail to load liblldb"); + throw std::runtime_error(std::string("fail to load ") + lldbPath); #endif // TODO: someone should free this. diff --git a/core/adapters/localwindowskerneladapter.cpp b/core/adapters/localwindowskerneladapter.cpp index 93ba7b5..b26a347 100644 --- a/core/adapters/localwindowskerneladapter.cpp +++ b/core/adapters/localwindowskerneladapter.cpp @@ -13,7 +13,6 @@ LocalWindowsKernelAdapter::LocalWindowsKernelAdapter(BinaryView* data) : DbgEngA bool LocalWindowsKernelAdapter::ExecuteWithArgsInternal(const std::string& path, const std::string& args, const std::string& workingDir, const LaunchConfigurations& configs) { - LogWarn("here"); m_aboutToBeKilled = false; if (this->m_debugActive) { diff --git a/core/adapters/windowskerneladapter.h b/core/adapters/windowskerneladapter.h index 2f2f0b8..d61fc39 100644 --- a/core/adapters/windowskerneladapter.h +++ b/core/adapters/windowskerneladapter.h @@ -31,8 +31,6 @@ namespace BinaryNinjaDebugger { bool Detach() override; bool Quit() override; - -// std::string GetDbgEngPath(const std::string& arch) override; }; class WindowsKernelAdapterType : public DebugAdapterType diff --git a/core/debugger.cpp b/core/debugger.cpp index 060d64f..1cfde7f 100644 --- a/core/debugger.cpp +++ b/core/debugger.cpp @@ -30,6 +30,10 @@ using namespace BinaryNinjaDebugger; void InitDebugAdapterTypes() { #ifdef WIN32 + if (!DbgEngAdapter::LoadDngEngLibraries()) + { + LogWarn("Failed to load DbgEng DLLs, the DbgEng adapter cannot work properly"); + } InitDbgEngAdapterType(); InitDbgEngTTDAdapterType(); InitWindowsKernelAdapterType(); @@ -91,6 +95,22 @@ static void RegisterSettings() "description" : "Path of the x86 DbgEng Installation. This folder should contain an x86 dbgeng.dll.", "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] })"); + settings->RegisterSetting("debugger.checkDbgEngDLLPath", + R"({ + "title" : "Check DbgEng DLL path", + "type" : "boolean", + "default" : true, + "description" : "Check if the DbgEng DLLs are loaded from the correct path. This ensures the debugger is using the correct version of DLLs", + "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] + })"); + settings->RegisterSetting("debugger.tryUnloadWrongDbgEngDLL", + R"({ + "title" : "Attempt to unload the DLL with wrong path", + "type" : "boolean", + "default" : false, + "description" : "Attempt to unload the already loaded DLL if they are from a wrong path. You may turn this on if the DbgEng DLLs, e.g., dbghelp.dll, is loaded from a wrong path, but it happens early than the debugger initialization", + "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] + })"); #endif settings->RegisterSetting("debugger.stackVariableAnnotations",