[MachO] Avoid leaking MachoObjCProcessor

This would leak if parsing of CFStrings was enabled while parsing of
Objective-C metadata was disabled.

It would also leak if exceptions were thrown or early returns were taken
in the ~500 lines between where the object was allocated and it was
deleted.

3 files changed, 33 insertions, 36 deletions

diff --git a/objectivec/objc.cpp b/objectivec/objc.cpp
index 6316c11f..f0a6a8f3 100644
--- a/objectivec/objc.cpp
+++ b/objectivec/objc.cpp
@@ -1713,7 +1713,7 @@ void ObjCProcessor::ProcessCFStrings()
 
 void ObjCProcessor::ProcessNSConstantArrays()
 {
-	m_symbolQueue = new SymbolQueue();
+	auto guard = ScopedSymbolQueue::Make();
 	uint64_t ptrSize = m_data->GetAddressSize();
 
 	auto idType = Type::NamedType(m_data, m_typeNames.id);
@@ -1742,16 +1742,16 @@ void ObjCProcessor::ProcessNSConstantArrays()
 				fmt::format("nsarray_{:x}", i), i, true);
 		}
 		auto id = m_data->BeginUndoActions();
-		m_symbolQueue->Process();
+		ScopedSymbolQueue::Get().Process();
 		m_data->EndBulkModifySymbols();
 		m_data->ForgetUndoActions(id);
 	}
-	delete m_symbolQueue;
+	
 }
 
 void ObjCProcessor::ProcessNSConstantDictionaries()
 {
-	m_symbolQueue = new SymbolQueue();
+	auto guard = ScopedSymbolQueue::Make();
 	uint64_t ptrSize = m_data->GetAddressSize();
 
 	auto idType = Type::NamedType(m_data, m_typeNames.id);
@@ -1786,16 +1786,15 @@ void ObjCProcessor::ProcessNSConstantDictionaries()
 				fmt::format("nsdict_{:x}", i), i, true);
 		}
 		auto id = m_data->BeginUndoActions();
-		m_symbolQueue->Process();
+		ScopedSymbolQueue::Get().Process();
 		m_data->EndBulkModifySymbols();
 		m_data->ForgetUndoActions(id);
 	}
-	delete m_symbolQueue;
 }
 
 void ObjCProcessor::ProcessNSConstantIntegerNumbers()
 {
-	m_symbolQueue = new SymbolQueue();
+	auto guard = ScopedSymbolQueue::Make();
 	uint64_t ptrSize = m_data->GetAddressSize();
 
 	StructureBuilder nsConstantIntegerNumberBuilder;
@@ -1844,11 +1843,10 @@ void ObjCProcessor::ProcessNSConstantIntegerNumbers()
 			}
 		}
 		auto id = m_data->BeginUndoActions();
-		m_symbolQueue->Process();
+		ScopedSymbolQueue::Get().Process();
 		m_data->EndBulkModifySymbols();
 		m_data->ForgetUndoActions(id);
 	}
-	delete m_symbolQueue;
 }
 
 void ObjCProcessor::ProcessNSConstantFloatingPointNumbers()
@@ -1893,7 +1891,7 @@ void ObjCProcessor::ProcessNSConstantFloatingPointNumbers()
 		if (!numbers)
 			continue;
 
-		m_symbolQueue = new SymbolQueue();
+		auto guard = ScopedSymbolQueue::Make();
 		auto start = numbers->GetStart();
 		auto end = numbers->GetEnd();
 		auto typeWidth = Type::NamedType(m_data, m_typeNames.nsConstantDoubleNumber)->GetWidth();
@@ -1935,16 +1933,15 @@ void ObjCProcessor::ProcessNSConstantFloatingPointNumbers()
 			DefineObjCSymbol(DataSymbol, Type::NamedType(m_data, *typeName), name, i, true);
 		}
 		auto id = m_data->BeginUndoActions();
-		m_symbolQueue->Process();
+		ScopedSymbolQueue::Get().Process();
 		m_data->EndBulkModifySymbols();
 		m_data->ForgetUndoActions(id);
-		delete m_symbolQueue;
 	}
 }
 
 void ObjCProcessor::ProcessNSConstantDatas()
 {
-	m_symbolQueue = new SymbolQueue();
+	auto guard = ScopedSymbolQueue::Make();
 	uint64_t ptrSize = m_data->GetAddressSize();
 
 	StructureBuilder nsConstantDataBuilder;
@@ -1972,11 +1969,10 @@ void ObjCProcessor::ProcessNSConstantDatas()
 				DataSymbol, Type::NamedType(m_data, m_typeNames.nsConstantData), fmt::format("nsdata_{:x}", i), i, true);
 		}
 		auto id = m_data->BeginUndoActions();
-		m_symbolQueue->Process();
+		ScopedSymbolQueue::Get().Process();
 		m_data->EndBulkModifySymbols();
 		m_data->ForgetUndoActions(id);
 	}
-	delete m_symbolQueue;
 }
 
 void ObjCProcessor::AddRelocatedPointer(uint64_t location, uint64_t rewrite)
diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp
index 9244a9c9..77d8d8fe 100644
--- a/view/macho/machoview.cpp
+++ b/view/macho/machoview.cpp
@@ -1856,9 +1856,11 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 		parseObjCStructs = false;
 	if (!GetSectionByName("__cfstring"))
 		parseCFStrings = false;
+
+	std::unique_ptr<MachoObjCProcessor> objcProcessor;
 	if (parseObjCStructs || parseCFStrings)
 	{
-		m_objcProcessor = new MachoObjCProcessor(this);
+		objcProcessor = std::make_unique<MachoObjCProcessor>(this);
 	}
 	if (parseObjCStructs)
 	{
@@ -2067,7 +2069,7 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 	{
 		// Add functions for all function symbols
 		m_logger->LogDebug("Parsing symbol table\n");
-		ParseSymbolTable(reader, header, header.symtab, indirectSymbols);
+		ParseSymbolTable(reader, header, header.symtab, indirectSymbols, objcProcessor.get());
 	}
 	catch (std::exception&)
 	{
@@ -2088,8 +2090,8 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 		uint64_t slidTarget = target + m_imageBaseAdjustment;
 		relocation.address = slidTarget;
 		DefineRelocation(m_arch, relocation, slidTarget, relocationLocation);
-		if (m_objcProcessor)
-			m_objcProcessor->AddRelocatedPointer(relocationLocation, slidTarget);
+		if (objcProcessor)
+			objcProcessor->AddRelocatedPointer(relocationLocation, slidTarget);
 	}
 	for (auto& [relocation, name] : header.externalRelocations)
 	{
@@ -2369,7 +2371,7 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 	if (parseCFStrings)
 	{
 		try {
-			m_objcProcessor->ProcessObjCLiterals();
+			objcProcessor->ProcessObjCLiterals();
 		}
 		catch (std::exception& ex)
 		{
@@ -2381,14 +2383,13 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_
 	if (parseObjCStructs)
 	{
 		try {
-			m_objcProcessor->ProcessObjCData();
+			objcProcessor->ProcessObjCData();
 		}
 		catch (std::exception& ex)
 		{
 			m_logger->LogError("Failed to process Objective-C Metadata. Binary may be malformed");
 			m_logger->LogError("Error: %s", ex.what());
 		}
-		delete m_objcProcessor;
 	}
 
 
@@ -2976,7 +2977,8 @@ void MachoView::ParseDynamicTable(BinaryReader& reader, MachOHeader& header, BNS
 }
 
 
-void MachoView::ParseSymbolTable(BinaryReader& reader, MachOHeader& header, const symtab_command& symtab, const vector<uint32_t>& indirectSymbols)
+void MachoView::ParseSymbolTable(BinaryReader& reader, MachOHeader& header, const symtab_command& symtab,
+	const vector<uint32_t>& indirectSymbols, MachoObjCProcessor* objcProcessor)
 {
 	if (header.ident.filetype == MH_DSYM)
 	{
@@ -3004,12 +3006,12 @@ void MachoView::ParseSymbolTable(BinaryReader& reader, MachOHeader& header, cons
 		if (header.chainedFixupsPresent)
 		{
 			m_logger->LogDebug("Chained Fixups");
-			ParseChainedFixups(header, header.chainedFixups);
+			ParseChainedFixups(header, header.chainedFixups, objcProcessor);
 		}
 		else if (header.chainStartsPresent)
 		{
 			m_logger->LogDebug("Chained Starts");
-			ParseChainedStarts(header, header.chainStarts);
+			ParseChainedStarts(header, header.chainStarts, objcProcessor);
 		}
 		if (header.exportTriePresent && header.isMainHeader)
 			ParseExportTrie(reader, header.exportTrie);
@@ -3197,7 +3199,8 @@ void MachoView::ParseSymbolTable(BinaryReader& reader, MachOHeader& header, cons
 }
 
 
-void MachoView::ParseChainedFixups(MachOHeader& header, linkedit_data_command chainedFixups)
+void MachoView::ParseChainedFixups(
+	MachOHeader& header, linkedit_data_command chainedFixups, MachoObjCProcessor* objcProcessor)
 {
 	if (!chainedFixups.dataoff)
 		return;
@@ -3596,9 +3599,9 @@ void MachoView::ParseChainedFixups(MachOHeader& header, linkedit_data_command ch
 							reloc.address = GetStart() + (chainEntryAddress - m_universalImageOffset);
 							DefineRelocation(m_arch, reloc, entryOffset, reloc.address);
 
-							if (m_objcProcessor)
+							if (objcProcessor)
 							{
-								m_objcProcessor->AddRelocatedPointer(reloc.address, entryOffset);
+								objcProcessor->AddRelocatedPointer(reloc.address, entryOffset);
 							}
 						}
 
@@ -3627,7 +3630,7 @@ void MachoView::ParseChainedFixups(MachOHeader& header, linkedit_data_command ch
 }
 
 
-void MachoView::ParseChainedStarts(MachOHeader& header, section_64 chainedStarts)
+void MachoView::ParseChainedStarts(MachOHeader& header, section_64 chainedStarts, MachoObjCProcessor* objcProcessor)
 {
 	if (!chainedStarts.offset)
 		return;
@@ -3799,9 +3802,9 @@ void MachoView::ParseChainedStarts(MachOHeader& header, section_64 chainedStarts
 					DefineRelocation(m_arch, reloc, entryOffset, reloc.address);
 					m_logger->LogDebug("Chained Starts: Adding relocated pointer %llx -> %llx", reloc.address, entryOffset);
 
-					if (m_objcProcessor)
+					if (objcProcessor)
 					{
-						m_objcProcessor->AddRelocatedPointer(reloc.address, entryOffset);
+						objcProcessor->AddRelocatedPointer(reloc.address, entryOffset);
 					}
 				}
 
diff --git a/view/macho/machoview.h b/view/macho/machoview.h
index dfbb24fe..253af9d1 100644
--- a/view/macho/machoview.h
+++ b/view/macho/machoview.h
@@ -1456,8 +1456,6 @@ namespace BinaryNinja
 			QualifiedName filesetEntryCommandQualName;
 		} m_typeNames;
 
-		MachoObjCProcessor* m_objcProcessor = nullptr;
-
 		uint64_t m_universalImageOffset;
 		bool m_parseOnly, m_backedByDatabase;
 		int64_t m_imageBaseAdjustment;
@@ -1488,7 +1486,7 @@ namespace BinaryNinja
 		void RebaseThreadStarts(BinaryReader& virtualReader, std::vector<uint32_t>& threadStarts, uint64_t stepMultiplier);
 		Ref<Symbol> DefineMachoSymbol(
 			BNSymbolType type, const std::string& name, uint64_t addr, BNSymbolBinding binding, bool deferred);
-		void ParseSymbolTable(BinaryReader& reader, MachOHeader& header, const symtab_command& symtab, const std::vector<uint32_t>& symbolStubsList);
+		void ParseSymbolTable(BinaryReader& reader, MachOHeader& header, const symtab_command& symtab, const std::vector<uint32_t>& symbolStubsList, MachoObjCProcessor*);
 		bool IsValidFunctionStart(uint64_t addr);
 		void ParseFunctionStarts(Platform* platform, uint64_t textBase, function_starts_command functionStarts);
 		bool ParseRelocationEntry(const relocation_info& info, uint64_t start, BNRelocationInfo& result);
@@ -1503,8 +1501,8 @@ namespace BinaryNinja
 			BNSymbolBinding binding);
 		bool GetSectionPermissions(MachOHeader& header, uint64_t address, uint32_t &flags);
 		bool GetSegmentPermissions(MachOHeader& header, uint64_t address, uint32_t &flags);
-		void ParseChainedFixups(MachOHeader& header, linkedit_data_command chainedFixups);
-		void ParseChainedStarts(MachOHeader& header, section_64 chainedStarts);
+		void ParseChainedFixups(MachOHeader& header, linkedit_data_command chainedFixups, MachoObjCProcessor*);
+		void ParseChainedStarts(MachOHeader& header, section_64 chainedStarts, MachoObjCProcessor*);
 
 		virtual uint64_t PerformGetEntryPoint() const override;