Fix a number of leaks and bad uses of free in the api

See #4751

architecture.cpp

@@ -2429,7 +2429,13 @@ vector<DisassemblyTextLine> DisassemblyTextRenderer::PostProcessInstructionTextL
 	size_t count = 0;
 	result = BNPostProcessDisassemblyTextRendererLines(
 	    m_object, addr, len, inLines, lines.size(), &count, indentSpaces.c_str());
-	BNFreeDisassemblyTextLines(inLines, lines.size());
+
+	for (size_t i = 0; i < lines.size(); i++)
+	{
+		InstructionTextToken::FreeInstructionTextTokenList(inLines[i].tokens, inLines[i].count);
+		Tag::FreeTagList(inLines[i].tags, inLines[i].tagCount);
+	}
+	delete[] inLines;
 
 	vector<DisassemblyTextLine> outLines;
 	for (size_t i = 0; i < count; i++)

binaryninjaapi.h

@@ -3625,6 +3625,7 @@ namespace BinaryNinja {
 
 		static BNTag** CreateTagList(const std::vector<Ref<Tag>>& tags, size_t* count);
 		static std::vector<Ref<Tag>> ConvertTagList(BNTag** tags, size_t count);
+		static void FreeTagList(BNTag** tags, size_t count);
 		static std::vector<Ref<Tag>> ConvertAndFreeTagList(BNTag** tags, size_t count);
 	};
 
@@ -3655,6 +3656,7 @@ namespace BinaryNinja {
 
 		static BNTagReference* CreateTagReferenceList(const std::vector<TagReference>& tags, size_t* count);
 		static std::vector<TagReference> ConvertTagReferenceList(BNTagReference* tags, size_t count);
+		static void FreeTagReferenceList(BNTagReference* tags, size_t count);
 		static std::vector<TagReference> ConvertAndFreeTagReferenceList(BNTagReference* tags, size_t count);
 	};
 
@@ -9470,6 +9472,7 @@ namespace BinaryNinja {
 
 		static PossibleValueSet FromAPIObject(BNPossibleValueSet& value);
 		BNPossibleValueSet ToAPIObject();
+		static void FreeAPIObject(BNPossibleValueSet* value);
 	};
 
 	class FlowGraph;
@@ -15255,6 +15258,7 @@ namespace BinaryNinja {
 		    BNType* type, const BNInstructionTextToken* prefix, size_t prefixCount, size_t width, size_t* count,
 		    BNTypeContext* typeCxt, size_t ctxCount);
 		static void FreeCallback(void* ctxt);
+		static void FreeLinesCallback(void* ctxt, BNDisassemblyTextLine* lines, size_t count);
 
 	  public:
 		DataRenderer();

binaryninjacore.h

@@ -37,14 +37,14 @@
 // Current ABI version for linking to the core. This is incremented any time
 // there are changes to the API that affect linking, including new functions,
 // new types, or modifications to existing functions or types.
-#define BN_CURRENT_CORE_ABI_VERSION 41
+#define BN_CURRENT_CORE_ABI_VERSION 42
 
 // Minimum ABI version that is supported for loading of plugins. Plugins that
 // are linked to an ABI version less than this will not be able to load and
 // will require rebuilding. The minimum version is increased when there are
 // incompatible changes that break binary compatibility, such as changes to
 // existing types or functions.
-#define BN_MINIMUM_CORE_ABI_VERSION 41
+#define BN_MINIMUM_CORE_ABI_VERSION 42
 
 #ifdef __GNUC__
 	#ifdef BINARYNINJACORE_LIBRARY
@@ -2791,6 +2791,7 @@ extern "C"
 		BNDisassemblyTextLine* (*getLinesForData)(void* ctxt, BNBinaryView* view, uint64_t addr, BNType* type,
 		    const BNInstructionTextToken* prefix, size_t prefixCount, size_t width, size_t* count,
 		    BNTypeContext* typeCtx, size_t ctxCount);
+		void (*freeLines)(void* ctx, BNDisassemblyTextLine* lines, size_t count);
 	} BNCustomDataRenderer;
 
 	typedef enum BNSegmentFlag

binaryview.cpp

@@ -769,6 +769,13 @@ std::vector<Ref<Tag>> Tag::ConvertTagList(BNTag** tags, size_t count)
 }
 
 
+void Tag::FreeTagList(BNTag** tags, size_t count)
+{
+	delete[] tags;
+	(void)count;
+}
+
+
 std::vector<Ref<Tag>> Tag::ConvertAndFreeTagList(BNTag** tags, size_t count)
 {
 	auto result = ConvertTagList(tags, count);
@@ -859,6 +866,13 @@ std::vector<TagReference> TagReference::ConvertTagReferenceList(BNTagReference*
 }
 
 
+void TagReference::FreeTagReferenceList(BNTagReference* tags, size_t count)
+{
+	delete[] tags;
+	(void)count;
+}
+
+
 std::vector<TagReference> TagReference::ConvertAndFreeTagReferenceList(BNTagReference* tags, size_t count)
 {
 	auto result = ConvertTagReferenceList(tags, count);
@@ -3621,6 +3635,7 @@ bool BinaryView::ParseTypeString(const string& source, map<QualifiedName, Ref<Ty
 		errors = errorStr;
 		BNFreeString(errorStr);
 	}
+	delete[] typesList.names;
 	if (!ok)
 		return false;
 
@@ -3675,6 +3690,7 @@ bool BinaryView::ParseTypesFromSource(const string& source, const vector<string>
 		errors = errorStr;
 		BNFreeString(errorStr);
 	}
+	delete[] typesList.names;
 	if (!ok)
 		return false;
 

datarenderer.cpp

@@ -17,6 +17,7 @@ DataRenderer::DataRenderer()
 	renderer.freeObject = FreeCallback;
 	renderer.isValidForData = IsValidForDataCallback;
 	renderer.getLinesForData = GetLinesForDataCallback;
+	renderer.freeLines = FreeLinesCallback;
 	AddRefForRegistration();
 	m_object = BNCreateDataRenderer(&renderer);
 }
@@ -106,6 +107,12 @@ void DataRenderer::FreeCallback(void* ctxt)
 }
 
 
+void DataRenderer::FreeLinesCallback(void* ctxt, BNDisassemblyTextLine* lines, size_t count)
+{
+	delete[] lines;
+}
+
+
 bool DataRenderer::IsValidForData(BinaryView* data, uint64_t addr, Type* type, vector<pair<Type*, size_t>>& context)
 {
 	BNTypeContext* typeCtx = new BNTypeContext[context.size()];

debuginfo.cpp

@@ -276,20 +276,20 @@ bool DebugInfo::AddType(const string& name, Ref<Type> type)
 
 bool DebugInfo::AddFunction(const DebugFunctionInfo& function)
 {
-	BNDebugFunctionInfo* input = new BNDebugFunctionInfo();
+	BNDebugFunctionInfo input;
 
-	input->shortName = function.shortName.size() ? BNAllocString(function.shortName.c_str()) : nullptr;
-	input->fullName = function.fullName.size() ? BNAllocString(function.fullName.c_str()) : nullptr;
-	input->rawName = function.rawName.size() ? BNAllocString(function.rawName.c_str()) : nullptr;
-	input->address = function.address;
-	input->type = function.type ? function.type->GetObject() : nullptr;
-	input->platform = function.platform ? function.platform->GetObject() : nullptr;
+	input.shortName = function.shortName.size() ? BNAllocString(function.shortName.c_str()) : nullptr;
+	input.fullName = function.fullName.size() ? BNAllocString(function.fullName.c_str()) : nullptr;
+	input.rawName = function.rawName.size() ? BNAllocString(function.rawName.c_str()) : nullptr;
+	input.address = function.address;
+	input.type = function.type ? function.type->GetObject() : nullptr;
+	input.platform = function.platform ? function.platform->GetObject() : nullptr;
 
-	bool result = BNAddDebugFunction(m_object, input);
+	bool result = BNAddDebugFunction(m_object, &input);
 
-	BNFreeString(input->shortName);
-	BNFreeString(input->fullName);
-	BNFreeString(input->rawName);
+	BNFreeString(input.shortName);
+	BNFreeString(input.fullName);
+	BNFreeString(input.rawName);
 
 	return result;
 }

flowgraphnode.cpp

@@ -131,7 +131,13 @@ void FlowGraphNode::SetLines(const vector<DisassemblyTextLine>& lines)
 	}
 
 	BNSetFlowGraphNodeLines(m_object, buf, lines.size());
-	BNFreeDisassemblyTextLines(buf, lines.size());
+
+	for (size_t i = 0; i < lines.size(); i++)
+	{
+		InstructionTextToken::FreeInstructionTextTokenList(buf[i].tokens, buf[i].count);
+		Tag::FreeTagList(buf[i].tags, buf[i].tagCount);
+	}
+	delete[] buf;
 
 	m_cachedLines = lines;
 	m_cachedLinesValid = true;

function.cpp

@@ -538,6 +538,31 @@ BNPossibleValueSet PossibleValueSet::ToAPIObject()
 }
 
 
+void PossibleValueSet::FreeAPIObject(BNPossibleValueSet* value)
+{
+	switch (value->state)
+	{
+	case SignedRangeValue:
+	case UnsignedRangeValue:
+		delete[] value->ranges;
+		break;
+	case LookupTableValue:
+		for (size_t i = 0; i < value->count; i ++)
+		{
+			delete[] value->table[i].fromValues;
+		}
+		delete[] value->table;
+		break;
+	case InSetOfValues:
+	case NotInSetOfValues:
+		delete[] value->valueSet;
+		break;
+	default:
+		break;
+	}
+}
+
+
 DataBuffer Function::GetConstantData(BNRegisterValueType state, uint64_t value, size_t size)
 {
 	return DataBuffer(BNGetConstantData(m_object, state, value, size));
@@ -2533,6 +2558,8 @@ void Function::SetUserVariableValue(const Variable& var, uint64_t defAddr, Possi
 	auto valueObj = value.ToAPIObject();
 
 	BNSetUserVariableValue(m_object, &var_data, &defSite, &valueObj);
+
+	PossibleValueSet::FreeAPIObject(&valueObj);
 }
 
 

interaction.cpp

@@ -661,7 +661,10 @@ bool BinaryNinja::GetFormInput(vector<FormInputField>& fields, const string& tit
 
 	// If user cancelled, there are no results
 	if (!ok)
+	{
+		delete[] fieldBuf;
 		return false;
+	}
 
 	// Copy results to API structures
 	for (size_t i = 0; i < fields.size(); i++)
@@ -689,7 +692,10 @@ bool BinaryNinja::GetFormInput(vector<FormInputField>& fields, const string& tit
 		}
 	}
 
+	// Free core-allocated results
 	BNFreeFormInputResults(fieldBuf, fields.size());
+
+	delete[] fieldBuf;
 	return true;
 }
 

metadata.cpp

@@ -55,6 +55,7 @@ Metadata::Metadata(const std::vector<Ref<Metadata>>& data)
 		dataList[i] = data[i]->m_object;
 
 	m_object = BNCreateMetadataArray(dataList, data.size());
+	delete[] dataList;
 }
 
 Metadata::Metadata(const std::map<std::string, Ref<Metadata>>& data)

python/datarender.py

@@ -91,6 +91,7 @@ def __init__(self, context=None):
 		self._cb.freeObject = self._cb.freeObject.__class__(self._free_object)
 		self._cb.isValidForData = self._cb.isValidForData.__class__(self._is_valid_for_data)
 		self._cb.getLinesForData = self._cb.getLinesForData.__class__(self._get_lines_for_data)
+		self._cb.freeLines = self._cb.freeLines.__class__(self._free_lines)
 		self.handle = core.BNCreateDataRenderer(self._cb)
 
 	@staticmethod
@@ -145,7 +146,7 @@ def _get_lines_for_data(self, ctxt, view, addr, type, prefix, prefixCount, width
 			result = self.perform_get_lines_for_data(ctxt, view, addr, type, prefixTokens, width, pycontext)
 
 			count[0] = len(result)
-			line_buf = (core.BNDisassemblyTextLine * len(result))()
+			self.line_buf = (core.BNDisassemblyTextLine * len(result))()
 			for i in range(len(result)):
 				line = result[i]
 				color = line.highlight
@@ -154,27 +155,30 @@ def _get_lines_for_data(self, ctxt, view, addr, type, prefix, prefixCount, width
 					raise ValueError("Specified color is not one of HighlightStandardColor, highlight.HighlightColor")
 				if isinstance(color, enums.HighlightStandardColor):
 					color = highlight.HighlightColor(color)
-				line_buf[i].highlight = color._to_core_struct()
+				self.line_buf[i].highlight = color._to_core_struct()
 				if line.address is None:
 					if len(line.tokens) > 0:
-						line_buf[i].addr = line.tokens[0].address
+						self.line_buf[i].addr = line.tokens[0].address
 					else:
-						line_buf[i].addr = 0
+						self.line_buf[i].addr = 0
 				else:
-					line_buf[i].addr = line.address
+					self.line_buf[i].addr = line.address
 				if line.il_instruction is not None:
-					line_buf[i].instrIndex = line.il_instruction.instr_index
+					self.line_buf[i].instrIndex = line.il_instruction.instr_index
 				else:
-					line_buf[i].instrIndex = 0xffffffffffffffff
+					self.line_buf[i].instrIndex = 0xffffffffffffffff
 
-				line_buf[i].count = len(line.tokens)
-				line_buf[i].tokens = function.InstructionTextToken._get_core_struct(line.tokens)
+				self.line_buf[i].count = len(line.tokens)
+				self.line_buf[i].tokens = function.InstructionTextToken._get_core_struct(line.tokens)
 
-			return ctypes.cast(line_buf, ctypes.c_void_p).value
+			return ctypes.cast(self.line_buf, ctypes.c_void_p).value
 		except:
 			log_error(traceback.format_exc())
 			return None
 
+	def _free_lines(self, ctxt, lines, count):
+		self.line_buf = None
+
 	def perform_free_object(self, ctxt):
 		pass
 

relocationhandler.cpp

@@ -139,6 +139,7 @@ bool CoreRelocationHandler::GetRelocationInfo(
 	    BNRelocationHandlerGetRelocationInfo(m_object, view->GetObject(), arch->GetObject(), results, result.size());
 	for (size_t i = 0; i < result.size(); i++)
 		result[i] = results[i];
+	delete[] results;
 	return status;
 }
 

type.cpp

@@ -1909,6 +1909,7 @@ TypeBuilder& TypeBuilder::SetParameters(const std::vector<FunctionParameter>& pa
 	size_t paramCount = 0;
 	BNFunctionParameter* paramArray = GetParamArray(params, paramCount);
 	BNSetFunctionTypeBuilderParameters(m_object, paramArray, paramCount);
+	delete[] paramArray;
 	return *this;
 }