From fd40266f767e51e649fb48376e25f47a60d79765 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Mon, 11 May 2026 18:49:26 -0700 Subject: Fix incorrect reference counting in C++ API --- activity.cpp | 2 +- binaryninjaapi.h | 2 +- binaryview.cpp | 6 +++--- debuginfo.cpp | 5 ++--- externallibrary.cpp | 7 +++++-- firmwareninja.cpp | 17 ++++++---------- pluginmanager.cpp | 2 +- typecontainer.cpp | 11 +++++++++- typeparser.cpp | 4 +++- workflow.cpp | 58 ++++++++++++++++++++--------------------------------- 10 files changed, 54 insertions(+), 60 deletions(-) diff --git a/activity.cpp b/activity.cpp index 0bb5858f..2b5ef140 100644 --- a/activity.cpp +++ b/activity.cpp @@ -19,7 +19,7 @@ Activity::Activity(const string& configuration, const std::function m_view; Ref m_function; + std::string PostRawRequest(const char* request); bool PostRequest(const std::string& command); public: @@ -21276,7 +21277,6 @@ namespace BinaryNinja { { public: FirmwareNinjaReferenceNode(BNFirmwareNinjaReferenceNode* node); - ~FirmwareNinjaReferenceNode(); /*! Returns true if the reference tree node contains a function diff --git a/binaryview.cpp b/binaryview.cpp index c1886ab9..6c4312c0 100644 --- a/binaryview.cpp +++ b/binaryview.cpp @@ -5728,7 +5728,7 @@ Ref BinaryView::GetExternalLibrary(const std::string& name) BNExternalLibrary* lib = BNBinaryViewGetExternalLibrary(m_object, name.c_str()); if (!lib) return nullptr; - return new ExternalLibrary(BNNewExternalLibraryReference(lib)); + return new ExternalLibrary(lib); } @@ -5759,7 +5759,7 @@ Ref BinaryView::AddExternalLocation(Ref sourceSymbol, if (!loc) return nullptr; - return new ExternalLocation(BNNewExternalLocationReference(loc)); + return new ExternalLocation(loc); } @@ -5774,7 +5774,7 @@ Ref BinaryView::GetExternalLocation(Ref sourceSymbol) BNExternalLocation* loc = BNBinaryViewGetExternalLocation(m_object, sourceSymbol->GetObject()); if (!loc) return nullptr; - return new ExternalLocation(BNNewExternalLocationReference(loc)); + return new ExternalLocation(loc); } diff --git a/debuginfo.cpp b/debuginfo.cpp index e0ab328c..3a337bc7 100644 --- a/debuginfo.cpp +++ b/debuginfo.cpp @@ -371,7 +371,7 @@ Ref DebugInfoParser::GetByName(const string& name) { BNDebugInfoParser* parser = BNGetDebugInfoParserByName(name.c_str()); if (parser) - return new DebugInfoParser(BNNewDebugInfoParserReference(parser)); + return new DebugInfoParser(parser); return nullptr; } @@ -472,6 +472,5 @@ bool CustomDebugInfoParser::ParseCallback(void* ctxt, BNDebugInfo* debugInfo, BN CustomDebugInfoParser::CustomDebugInfoParser(const string& name) : - DebugInfoParser( - BNNewDebugInfoParserReference(BNRegisterDebugInfoParser(name.c_str(), IsValidCallback, ParseCallback, this))) + DebugInfoParser(BNRegisterDebugInfoParser(name.c_str(), IsValidCallback, ParseCallback, this)) {} diff --git a/externallibrary.cpp b/externallibrary.cpp index a2e0573a..f1b34f1a 100644 --- a/externallibrary.cpp +++ b/externallibrary.cpp @@ -44,7 +44,7 @@ Ref ExternalLibrary::GetBackingFile() const BNProjectFile* file = BNExternalLibraryGetBackingFile(m_object); if (!file) return nullptr; - return new ProjectFile(BNNewProjectFileReference(file)); + return new ProjectFile(file); } @@ -81,7 +81,10 @@ std::optional ExternalLocation::GetTargetSymbol() { if (BNExternalLocationHasTargetSymbol(m_object)) { - return BNExternalLocationGetTargetSymbol(m_object); + char* sym = BNExternalLocationGetTargetSymbol(m_object); + std::string result = sym; + BNFreeString(sym); + return result; } return {}; } diff --git a/firmwareninja.cpp b/firmwareninja.cpp index 501ef2c5..d39923b5 100644 --- a/firmwareninja.cpp +++ b/firmwareninja.cpp @@ -63,7 +63,7 @@ FirmwareNinjaRelationship::FirmwareNinjaRelationship(Ref view, BNFir if (handle) m_object = handle; else - m_object = BNNewFirmwareNinjaRelationshipReference(BNCreateFirmwareNinjaRelationship(view->GetObject())); + m_object = BNCreateFirmwareNinjaRelationship(view->GetObject()); } @@ -204,7 +204,7 @@ Ref FirmwareNinjaRelationship::GetSecondaryExternalProjectFile() co if (!bnProjectFile) return nullptr; - return new ProjectFile(BNNewProjectFileReference(bnProjectFile)); + return new ProjectFile(bnProjectFile); } @@ -305,12 +305,6 @@ FirmwareNinjaReferenceNode::FirmwareNinjaReferenceNode(BNFirmwareNinjaReferenceN } -FirmwareNinjaReferenceNode::~FirmwareNinjaReferenceNode() -{ - BNFreeFirmwareNinjaReferenceNode(m_object); -} - - bool FirmwareNinjaReferenceNode::IsFunction() { return BNFirmwareNinjaReferenceNodeIsFunction(m_object); @@ -366,6 +360,7 @@ std::vector> FirmwareNinjaReferenceNode::GetChil BNNewFirmwareNinjaReferenceNodeReference(bnChildren[i]))); } + BNFreeFirmwareNinjaReferenceNodes(bnChildren, count); return result; } @@ -623,7 +618,7 @@ Ref FirmwareNinja::GetReferenceTree( if (!bnReferenceTree) return nullptr; - return new FirmwareNinjaReferenceNode(BNNewFirmwareNinjaReferenceNodeReference(bnReferenceTree)); + return new FirmwareNinjaReferenceNode(bnReferenceTree); } @@ -641,7 +636,7 @@ Ref FirmwareNinja::GetReferenceTree( if (!bnReferenceTree) return nullptr; - return new FirmwareNinjaReferenceNode(BNNewFirmwareNinjaReferenceNodeReference(bnReferenceTree)); + return new FirmwareNinjaReferenceNode(bnReferenceTree); } @@ -658,7 +653,7 @@ Ref FirmwareNinja::GetReferenceTree( if (!bnReferenceTree) return nullptr; - return new FirmwareNinjaReferenceNode(BNNewFirmwareNinjaReferenceNodeReference(bnReferenceTree)); + return new FirmwareNinjaReferenceNode(bnReferenceTree); } diff --git a/pluginmanager.cpp b/pluginmanager.cpp index 1ce77606..57eac985 100644 --- a/pluginmanager.cpp +++ b/pluginmanager.cpp @@ -297,7 +297,7 @@ bool Extension::AreDependenciesBeingInstalled() const string Extension::GetCreationDate() { - return BNPluginGetCurrentVersionCreationDate(m_object); + RETURN_STRING(BNPluginGetCurrentVersionCreationDate(m_object)); } string Extension::GetProjectData() diff --git a/typecontainer.cpp b/typecontainer.cpp index 3ecf6aa5..cbff21ee 100644 --- a/typecontainer.cpp +++ b/typecontainer.cpp @@ -80,6 +80,10 @@ TypeContainer::TypeContainer(const TypeContainer& other) TypeContainer& TypeContainer::operator=(const TypeContainer& other) { + if (this == &other) + return *this; + if (m_object) + BNFreeTypeContainer(m_object); m_object = BNDuplicateTypeContainer(other.m_object); return *this; } @@ -87,7 +91,11 @@ TypeContainer& TypeContainer::operator=(const TypeContainer& other) TypeContainer& TypeContainer::operator=(TypeContainer&& other) { - m_object = std::move(other.m_object); + if (this == &other) + return *this; + if (m_object) + BNFreeTypeContainer(m_object); + m_object = other.m_object; other.m_object = nullptr; return *this; } @@ -370,6 +378,7 @@ bool TypeContainer::ParseTypeString( if (!success) { + BNFreeQualifiedNameAndType(&apiResult); return false; } diff --git a/typeparser.cpp b/typeparser.cpp index 8fe20a1f..18ecac08 100644 --- a/typeparser.cpp +++ b/typeparser.cpp @@ -97,7 +97,9 @@ std::string TypeParser::FormatParseErrors(const std::vector& er BNFreeString(apiError.fileName); } - return string; + std::string result = string ? string : ""; + BNFreeString(string); + return result; } diff --git a/workflow.cpp b/workflow.cpp index 5be636d2..bf5b5579 100644 --- a/workflow.cpp +++ b/workflow.cpp @@ -375,11 +375,7 @@ bool WorkflowMachine::PostRequest(const std::string& command) rapidjson::Writer writer(buffer); request.Accept(writer); - string jsonResult; - if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), buffer.GetString()); - else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), buffer.GetString()); + string jsonResult = PostRawRequest(buffer.GetString()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -402,13 +398,23 @@ WorkflowMachine::WorkflowMachine(Ref function): m_function(function) } -bool WorkflowMachine::PostJsonRequest(const std::string& request) +string WorkflowMachine::PostRawRequest(const char* request) { - string jsonResult; + char* result; if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), request.c_str()); + result = BNPostWorkflowRequestForFunction(m_function->GetObject(), request); else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), request.c_str()); + result = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), request); + + string jsonResult(result); + BNFreeString(result); + return jsonResult; +} + + +bool WorkflowMachine::PostJsonRequest(const std::string& request) +{ + string jsonResult = PostRawRequest(request.c_str()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -450,11 +456,7 @@ WorkflowMachine::Status WorkflowMachine::GetStatus() rapidjson::Writer writer(buffer); request.Accept(writer); - string jsonResult; - if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), buffer.GetString()); - else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), buffer.GetString()); + string jsonResult = PostRawRequest(buffer.GetString()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -532,11 +534,7 @@ bool WorkflowMachine::SetLogEnabled(bool enable, bool global) rapidjson::Writer writer(buffer); request.Accept(writer); - string jsonResult; - if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), buffer.GetString()); - else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), buffer.GetString()); + string jsonResult = PostRawRequest(buffer.GetString()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -558,11 +556,7 @@ std::optional WorkflowMachine::QueryOverride(const string& activity) rapidjson::Writer writer(buffer); request.Accept(writer); - string jsonResult; - if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), buffer.GetString()); - else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), buffer.GetString()); + string jsonResult = PostRawRequest(buffer.GetString()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -585,11 +579,7 @@ bool WorkflowMachine::SetOverride(const string& activity, bool enable) rapidjson::Writer writer(buffer); request.Accept(writer); - string jsonResult; - if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), buffer.GetString()); - else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), buffer.GetString()); + string jsonResult = PostRawRequest(buffer.GetString()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -611,11 +601,7 @@ bool WorkflowMachine::ClearOverride(const string& activity) rapidjson::Writer writer(buffer); request.Accept(writer); - string jsonResult; - if (m_function) - jsonResult = BNPostWorkflowRequestForFunction(m_function->GetObject(), buffer.GetString()); - else - jsonResult = BNPostWorkflowRequestForBinaryView(m_view->GetObject(), buffer.GetString()); + string jsonResult = PostRawRequest(buffer.GetString()); rapidjson::Document response(rapidjson::kObjectType); response.Parse(jsonResult.c_str()); @@ -720,7 +706,7 @@ Ref Workflow::RegisterActivity(Ref activity, const vector Workflow::GetActivity(const string& activity) { BNActivity* activityObject = BNWorkflowGetActivity(m_object, activity.c_str()); - return new Activity(BNNewActivityReference(activityObject)); + return new Activity(activityObject); } -- cgit v1.3.1