diff --git a/config.cc b/config.cc index 4415446a..3764d8cc 100644 --- a/config.cc +++ b/config.cc @@ -45,12 +45,13 @@ namespace util = ::proto2::util; } // namespace google::protobuf #endif -using google::protobuf::util::JsonPrintOptions; using google::protobuf::util::JsonParseOptions; +using google::protobuf::util::JsonPrintOptions; using google::protobuf::util::JsonStringToMessage; using google::protobuf::util::MessageToJsonString; using security::binexport::GetCommonAppDataDirectory; using security::binexport::GetOrCreateAppDataDirectory; +using security::binexport::GetProgramFilesDirectory; namespace security::bindiff::config { @@ -185,4 +186,19 @@ void MergeInto(const Config& from, Config& config) { } } +std::string GetBinDiffDirOrDefault(const Config& from) { + if (!from.directory().empty()) { + return from.directory(); + } +#if defined(_WIN32) + return JoinPath(GetProgramFilesDirectory().value_or(R"(C:\Program Files)"), + absl::StrCat("BinDiff ", kBinDiffRelease)); +#elif defined(__APPLE__) + return JoinPath(*GetProgramFilesDirectory(), + "BinDiff/BinDiff.app/Contents/MacOS"); +#else + return JoinPath(*GetProgramFilesDirectory(), "bindiff"); +#endif +} + } // namespace security::bindiff::config diff --git a/config.h b/config.h index 0f81cf3b..63d7fd80 100644 --- a/config.h +++ b/config.h @@ -15,6 +15,8 @@ #ifndef CONFIG_H_ #define CONFIG_H_ +#include + #include "third_party/absl/base/attributes.h" #include "third_party/absl/status/status.h" #include "third_party/absl/status/statusor.h" @@ -51,6 +53,8 @@ absl::Status SaveUserConfig(const Config& config); // matter. void MergeInto(const Config& from, Config& config); +std::string GetBinDiffDirOrDefault(const Config& from); + } // namespace security::bindiff::config #endif // CONFIG_H_ diff --git a/ida/CMakeLists.txt b/ida/CMakeLists.txt index 95fe01e7..e10c2855 100644 --- a/ida/CMakeLists.txt +++ b/ida/CMakeLists.txt @@ -43,6 +43,8 @@ ida_target_link_libraries(${bindiff_ida_plugin_name} binexport_plugin_shared bindiff_base bindiff_shared + absl::cleanup + absl::function_ref absl::str_format absl::time ) diff --git a/ida/main_plugin.cc b/ida/main_plugin.cc index 4ef13232..b1a921ef 100644 --- a/ida/main_plugin.cc +++ b/ida/main_plugin.cc @@ -15,6 +15,7 @@ #include "third_party/zynamics/bindiff/ida/main_plugin.h" #include +#include #include #include #include @@ -283,26 +284,17 @@ void Plugin::VisualDiff(uint32_t index, bool call_graph_diff) { } LOG(INFO) << "Sending result to BinDiff GUI..."; - const auto& config = config::Proto(); - const auto& ui_config = config.ui(); - - const std::string default_bindiff_dir = -#if defined(_WIN32) - absl::StrCat("C:\\Program Files\\BinDiff ", kBinDiffRelease); -#elif defined(__APPLE__) - "/Applications/BinDiff/BinDiff.app/Contents/MacOS"; -#else - "/opt/bindiff"; -#endif - SendGuiMessage( - ui_config.retries() != 0 ? ui_config.retries() : 20, - !config.directory().empty() ? config.directory() : default_bindiff_dir, - !ui_config.server().empty() ? ui_config.server() : "127.0.0.1", - ui_config.port() != 0 ? ui_config.port() : 2000, message, - /*callback=*/nullptr); - } catch (const std::runtime_error& message) { - LOG(INFO) << "Error while calling BinDiff UI: " << message.what(); - warning("Error while calling BinDiff UI: %s\n", message.what()); + if (absl::Status status = SendGuiMessage(config::Proto(), message, [] {}); + !status.ok()) { + const std::string error_message = absl::StrCat( + "Cannot launch BinDiff user interface. Process creation failed: ", + status.message()); + LOG(INFO) << error_message; + warning("%s\n", error_message.c_str()); + } + } catch (const std::runtime_error& error_message) { + LOG(INFO) << "Error while calling BinDiff UI: " << error_message.what(); + warning("Error while calling BinDiff UI: %s\n", error_message.what()); } catch (...) { LOG(INFO) << "Unknown error while calling BinDiff UI"; warning("Unknown error while calling BinDiff UI\n"); @@ -319,7 +311,7 @@ bool Plugin::DiscardResults(Plugin::DiscardResultsKind kind) { ask_yn(ASKBTN_YES, "%sCurrent diff results have not been" " saved - save before closing?", - kind == DiscardResultsKind::kAskSave ? "HIDECANCEL\n" : ""); + kind == DiscardResultsKind::kAskSave ? "HIDECANCEL\n" : ""); if (answer == ASKBTN_YES) { DoSaveResults(); } else if (answer == ASKBTN_CANCEL) { @@ -560,10 +552,10 @@ bool DoDiffDatabase(bool filtered) { DiffAddressRange(start_address_source, end_address_source, start_address_target, end_address_target); if (!diffed.ok()) { - const std::string message = + const std::string error_message = absl::StrCat("Error while diffing: ", diffed.status().message()); - LOG(INFO) << message; - warning("%s\n", message.c_str()); + LOG(INFO) << error_message; + warning("%s\n", error_message.c_str()); return false; } return *diffed; @@ -625,9 +617,9 @@ bool DoPortComments() { start_address_source, end_address_source, start_address_target, end_address_target, min_confidence, min_similarity); if (!status.ok()) { - const std::string message(status.message()); - LOG(INFO) << "Error: " << message; - warning("Error: %s\n", message.c_str()); + const std::string error_message(status.message()); + LOG(INFO) << "Error: " << error_message; + warning("Error: %s\n", error_message.c_str()); return false; } MatchedFunctionsChooser::Refresh(); @@ -758,8 +750,7 @@ bool DoSaveResults() { absl::StatusOr filename = GetSaveFilename("Save Results As", default_filename, {{"BinDiff Result files", "*.BinDiff"}, - {"BinDiff Groundtruth files", "*.truth"} - }); + {"BinDiff Groundtruth files", "*.truth"}}); if (!filename.ok()) { return false; } @@ -775,10 +766,10 @@ bool DoSaveResults() { } if (!status.ok()) { - std::string message = + std::string error_message = absl::StrCat("Error writing results: ", status.message()); - LOG(INFO) << message; - warning("%s\n", message.c_str()); + LOG(INFO) << error_message; + warning("%s\n", error_message.c_str()); return false; } @@ -879,9 +870,9 @@ bool Plugin::LoadResults() { LOG(INFO) << absl::StrCat("done (", HumanReadableDuration(timer.elapsed()), ")"); return true; - } catch (const std::exception& message) { - LOG(INFO) << "Error loading results: " << message.what(); - warning("Error loading results: %s\n", message.what()); + } catch (const std::exception& error_message) { + LOG(INFO) << "Error loading results: " << error_message.what(); + warning("Error loading results: %s\n", error_message.what()); } catch (...) { LOG(INFO) << "Error loading results."; warning("Error loading results."); @@ -1021,9 +1012,9 @@ int HandleImportSymbolsComments(action_activation_ctx_t* context, absl::Status status = results->PortComments( absl::MakeConstSpan(&ida_selection.front(), ida_selection.size()), kind); if (!status.ok()) { - const std::string message(status.message()); - LOG(INFO) << "Error: " << message; - warning("Error: %s\n", message.c_str()); + const std::string error_message(status.message()); + LOG(INFO) << "Error: " << error_message; + warning("Error: %s\n", error_message.c_str()); return 0; } // Need to refresh all choosers @@ -1053,9 +1044,9 @@ int idaapi ConfirmMatchesAction::activate(action_activation_ctx_t* context) { absl::Status status = results->ConfirmMatches( absl::MakeConstSpan(&ida_selection.front(), ida_selection.size())); if (!status.ok()) { - const std::string message(status.message()); - LOG(INFO) << "Error: " << message; - warning("Error: %s\n", message.c_str()); + const std::string error_message(status.message()); + LOG(INFO) << "Error: " << error_message; + warning("Error: %s\n", error_message.c_str()); return 0; } MatchedFunctionsChooser::Refresh(); @@ -1065,9 +1056,9 @@ int idaapi ConfirmMatchesAction::activate(action_activation_ctx_t* context) { int HandleCopyAddress(Address address) { absl::Status status = CopyToClipboard(FormatAddress(address)); if (!status.ok()) { - const std::string message(status.message()); - LOG(INFO) << "Error: " << message; - warning("Error: %s\n", message.c_str()); + const std::string error_message(status.message()); + LOG(INFO) << "Error: " << error_message; + warning("Error: %s\n", error_message.c_str()); return 0; } return 1; @@ -1139,9 +1130,9 @@ class AddMatchAction : public ActionHandler { results->AddMatch(results->GetPrimaryAddress(index_primary), results->GetSecondaryAddress(index_secondary)); if (!status.ok()) { - const std::string message(status.message()); - LOG(INFO) << "Error: " << message; - warning("Error: %s\n", message.c_str()); + const std::string error_message(status.message()); + LOG(INFO) << "Error: " << error_message; + warning("Error: %s\n", error_message.c_str()); return 0; } MatchedFunctionsChooser::Refresh(); @@ -1199,13 +1190,15 @@ void Plugin::InitActions() { MatchedFunctionsChooser::RegisterActions(); - // Unmatched choosers + // Primary unmatched chooser register_action(CopyAddressAction::MakeActionDesc( UnmatchedFunctionsChooserPrimary::kCopyAddressAction, "Copy ~a~ddress", /*shortcut=*/"", /*tooltip=*/"", /*icon=*/-1)); register_action(AddMatchAction::MakeActionDesc( UnmatchedFunctionsChooserPrimary::kAddMatchAction, "Add ~m~atch", /*shortcut=*/"", /*tooltip=*/"", /*icon=*/-1)); + + // Secondary unmatched chooser register_action(CopyAddressAction::MakeActionDesc( UnmatchedFunctionsChooserSecondary::kCopyAddressAction, "Copy ~a~ddress", /*shortcut=*/"", /*tooltip=*/"", /*icon=*/-1)); diff --git a/ida/results.cc b/ida/results.cc index 94bda0a7..d27a8839 100644 --- a/ida/results.cc +++ b/ida/results.cc @@ -1120,7 +1120,7 @@ bool Results::PrepareVisualCallGraphDiff(size_t index, std::string* message) { return false; } - const FixedPointInfo& fixed_point_info(*indexed_fixed_points_[index]); + const FixedPointInfo& fixed_point_info = *indexed_fixed_points_[index]; ++diff_database_id_; std::string name = absl::StrCat("visual_diff", diff_database_id_, ".database"); @@ -1158,19 +1158,16 @@ bool Results::PrepareVisualDiff(size_t index, std::string* message) { return false; } - const FixedPointInfo& fixed_point_info(*indexed_fixed_points_[index]); + const FixedPointInfo& fixed_point_info = *indexed_fixed_points_[index]; - FlowGraphInfo empty; - const FlowGraphInfo& primary_info( - flow_graph_infos1_.find(fixed_point_info.primary) != - flow_graph_infos1_.end() - ? flow_graph_infos1_.find(fixed_point_info.primary)->second - : empty); - const FlowGraphInfo& secondary_info( - flow_graph_infos2_.find(fixed_point_info.secondary) != - flow_graph_infos2_.end() - ? flow_graph_infos2_.find(fixed_point_info.secondary)->second - : empty); + FlowGraphInfo empty{0}; + auto primary_entry = flow_graph_infos1_.find(fixed_point_info.primary); + const FlowGraphInfo& primary_info = + primary_entry != flow_graph_infos1_.end() ? primary_entry->second : empty; + auto secondary_entry = flow_graph_infos2_.find(fixed_point_info.secondary); + const FlowGraphInfo& secondary_info = + secondary_entry != flow_graph_infos2_.end() ? secondary_entry->second + : empty; if (primary_info.instruction_count == 0 && secondary_info.instruction_count == 0) { warning("Both functions are empty, nothing to display!"); @@ -1233,7 +1230,7 @@ FixedPoint* Results::FindFixedPoint(const FixedPointInfo& fixed_point_info) { return const_cast(&fixed_point); } } - return 0; + return nullptr; } void Results::Read(Reader* reader) { diff --git a/ida/visual_diff.cc b/ida/visual_diff.cc index d4ed9275..5b205f8f 100644 --- a/ida/visual_diff.cc +++ b/ida/visual_diff.cc @@ -34,8 +34,11 @@ #include #include #include -#include // NOLINT +#include // NOLINT +#include // NOLINT +#include "third_party/absl/cleanup/cleanup.h" +#include "third_party/absl/functional/function_ref.h" #include "third_party/absl/status/status.h" #include "third_party/absl/strings/str_cat.h" #include "third_party/absl/strings/string_view.h" @@ -46,18 +49,20 @@ #include "third_party/zynamics/bindiff/match/context.h" #include "third_party/zynamics/bindiff/start_ui.h" #include "third_party/zynamics/binexport/util/filesystem.h" +#include "third_party/zynamics/binexport/util/status_macros.h" namespace security::bindiff { -bool DoSendGuiMessageTCP(absl::string_view server, uint16_t port, - absl::string_view arguments) { +absl::Status DoSendGuiMessageTCP(absl::string_view server, uint16_t port, + absl::string_view arguments) { #ifdef _WIN32 static int winsock_status = []() -> int { WSADATA wsa_data; return WSAStartup(MAKEWORD(2, 2), &wsa_data); }(); if (winsock_status != 0) { - return false; + return absl::UnknownError( + absl::StrCat("WSAStartup failed with error: ", winsock_status)); } // Use the original BSD names for these. @@ -67,7 +72,7 @@ bool DoSendGuiMessageTCP(absl::string_view server, uint16_t port, }; #endif - uint32_t packet_size(arguments.size()); + uint32_t packet_size = arguments.size(); std::string packet(reinterpret_cast(&packet_size), reinterpret_cast(&packet_size) + 4); absl::StrAppend(&packet, arguments); @@ -81,9 +86,8 @@ bool DoSendGuiMessageTCP(absl::string_view server, uint16_t port, auto err = getaddrinfo(std::string(server).c_str(), absl::StrCat(port).c_str(), &hints, &address_info); if (err != 0) { - // TODO(cblichmann): This function should return a absl::Status and use - // gai_strerror(err). - return false; + return absl::UnknownError( + absl::StrCat("getaddrinfo(): ", gai_strerror(err))); } std::unique_ptr address_info_deleter(address_info, freeaddrinfo); @@ -102,46 +106,48 @@ bool DoSendGuiMessageTCP(absl::string_view server, uint16_t port, close(socket_fd); } if (!connected) { - return false; + return absl::UnknownError( + absl::StrCat("Cannot connect to ", server, ":", port)); } - bool success = - write(socket_fd, packet.data(), packet.size()) == packet.size(); - close(socket_fd); - return success; -} + absl::Cleanup socked_closer([socket_fd] { close(socket_fd); }); -bool SendGuiMessage(int retries, absl::string_view bindiff_dir, - absl::string_view server, uint16_t port, - absl::string_view arguments, - std::function callback) { - if (DoSendGuiMessageTCP(server, port, arguments)) { - return true; + ssize_t bytes_written = write(socket_fd, packet.data(), packet.size()); + if (bytes_written != packet.size()) { + return absl::UnknownError( + absl::StrCat("Failed to send ", packet.size(), " bytes to UI")); } - const auto& ui_config = config::Proto().ui(); - absl::Status status = StartUiWithOptions( - /*extra_args=*/{}, StartUiOptions{} - .set_java_binary(ui_config.java_binary()) - .set_java_vm_options(ui_config.java_vm_option()) - .set_max_heap_size_mb(ui_config.max_heap_size_mb()) - .set_bindiff_dir(std::string(bindiff_dir))); - if (!status.ok()) { - throw std::runtime_error{absl::StrCat( - "Cannot launch BinDiff user interface. Process creation failed: ", - status.message())}; + return absl::OkStatus(); +} + +absl::Status SendGuiMessage(const Config& config, absl::string_view arguments, + absl::FunctionRef on_retry_callback) { + const auto& ui_config = config.ui(); + const std::string server = + !ui_config.server().empty() ? ui_config.server() : "127.0.0.1"; + const uint16_t port = ui_config.port() != 0 ? ui_config.port() : 2000; + if (DoSendGuiMessageTCP(server, port, arguments).ok()) { + return absl::OkStatus(); } + NA_RETURN_IF_ERROR(StartUiWithOptions( + /*extra_args=*/{}, + StartUiOptions{} + .set_java_binary(ui_config.java_binary()) + .set_java_vm_options(ui_config.java_vm_option()) + .set_max_heap_size_mb(ui_config.max_heap_size_mb()) + .set_bindiff_dir(config::GetBinDiffDirOrDefault(config)))); + const int retries = ui_config.retries() != 0 ? ui_config.retries() : 20; + absl::Status status; for (int retry = 0; retry < retries * 10; ++retry) { - if (DoSendGuiMessageTCP(server, port, arguments)) { - return true; + if (status = DoSendGuiMessageTCP(server, port, arguments); status.ok()) { + return absl::OkStatus(); } absl::SleepFor(absl::Milliseconds(100)); - if (callback) { - callback(); - } + on_retry_callback(); } - return false; + return status; } } // namespace security::bindiff diff --git a/ida/visual_diff.h b/ida/visual_diff.h index b418325a..3fad1d18 100644 --- a/ida/visual_diff.h +++ b/ida/visual_diff.h @@ -18,15 +18,18 @@ #include #include +#include "third_party/absl/functional/function_ref.h" +#include "third_party/absl/status/status.h" #include "third_party/absl/strings/string_view.h" +#include "third_party/zynamics/bindiff/config.h" #include "third_party/zynamics/binexport/util/types.h" namespace security::bindiff { -bool SendGuiMessage(int retries, absl::string_view bindiff_dir, - absl::string_view server, uint16_t port, - absl::string_view arguments, - std::function callback); +// Sends a one-shot message to the Java UI, starting it if it is not already +// open. +absl::Status SendGuiMessage(const Config& config, absl::string_view arguments, + absl::FunctionRef on_retry_callback); } // namespace security::bindiff