Don't leak DataBuffers

`MMappedFileAccessor::ReadBuffer` was returning a heap-allocated
`DataBuffer`, but no callers were ever deleting it. There does not
appear to be any reason to heap allocate the `DataBuffer` as the type is
effectively a smart pointer wrapper around `BNDataBuffer`. Switch to
returning it by value instead.

Additionally, `MMappedFileAccessor::ReadBuffer` was allocating a buffer,
copying data into it, and then handing that allocation to the `DataBuffer`
constructor. The constructor copies data into a new allocation it
owns so this allocation is unnecessary and was being leaked.

3 files changed, 28 insertions, 30 deletions

diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp
index d30dbe14..a7c7ffb0 100644
--- a/view/sharedcache/core/SharedCache.cpp
+++ b/view/sharedcache/core/SharedCache.cpp
@@ -223,7 +223,7 @@ void SharedCache::PerformInitialLoad()
 
 	m_baseFilePath = path;
 
-	DataBuffer sig = *baseFile->ReadBuffer(0, 4);
+	DataBuffer sig = baseFile->ReadBuffer(0, 4);
 	if (sig.GetLength() != 4)
 		abort();
 	const char* magic = (char*)sig.GetData();
@@ -1571,13 +1571,13 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)
 
 				auto name = stubIsland.prettyName;
 				m_dscView->GetParentView()->GetParentView()->WriteBuffer(
-					m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
+					m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
 				m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, stubIsland.size, rawViewEnd, stubIsland.size,
 					SegmentReadable | SegmentExecutable);
 				m_dscView->AddUserSegment(stubIsland.start, stubIsland.size, rawViewEnd, stubIsland.size,
 					SegmentReadable | SegmentExecutable);
 				m_dscView->AddUserSection(name, stubIsland.start, stubIsland.size, ReadOnlyCodeSectionSemantics);
-				m_dscView->WriteBuffer(stubIsland.start, *buff);
+				m_dscView->WriteBuffer(stubIsland.start, buff);
 
 				stubIsland.loaded = true;
 
@@ -1611,13 +1611,13 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)
 
 				auto name = dyldData.prettyName;
 				m_dscView->GetParentView()->GetParentView()->WriteBuffer(
-					m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
-				m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
+					m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
+				m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
 				m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, dyldData.size, rawViewEnd, dyldData.size,
 					SegmentReadable);
 				m_dscView->AddUserSegment(dyldData.start, dyldData.size, rawViewEnd, dyldData.size, SegmentReadable);
 				m_dscView->AddUserSection(name, dyldData.start, dyldData.size, ReadOnlyDataSectionSemantics);
-				m_dscView->WriteBuffer(dyldData.start, *buff);
+				m_dscView->WriteBuffer(dyldData.start, buff);
 
 				dyldData.loaded = true;
 				dyldData.rawViewOffsetIfLoaded = rawViewEnd;
@@ -1650,12 +1650,12 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)
 
 				auto name = region.prettyName;
 				m_dscView->GetParentView()->GetParentView()->WriteBuffer(
-					m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
-				m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
+					m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
+				m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
 				m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, region.size, rawViewEnd, region.size, region.flags);
 				m_dscView->AddUserSegment(region.start, region.size, rawViewEnd, region.size, region.flags);
 				m_dscView->AddUserSection(name, region.start, region.size, ReadOnlyCodeSectionSemantics);
-				m_dscView->WriteBuffer(region.start, *buff);
+				m_dscView->WriteBuffer(region.start, buff);
 
 				region.loaded = true;
 				region.rawViewOffsetIfLoaded = rawViewEnd;
@@ -1685,13 +1685,13 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)
 	ParseAndApplySlideInfoForFile(targetFile);
 	auto buff = reader.ReadBuffer(targetSegment->start, targetSegment->size);
 	m_dscView->GetParentView()->GetParentView()->WriteBuffer(
-		m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
-	m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
+		m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
+	m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
 	m_dscView->GetParentView()->AddAutoSegment(
 		rawViewEnd, targetSegment->size, rawViewEnd, targetSegment->size, SegmentReadable);
 	m_dscView->AddUserSegment(
 		targetSegment->start, targetSegment->size, rawViewEnd, targetSegment->size, targetSegment->flags);
-	m_dscView->WriteBuffer(targetSegment->start, *buff);
+	m_dscView->WriteBuffer(targetSegment->start, buff);
 
 	targetSegment->loaded = true;
 	targetSegment->rawViewOffsetIfLoaded = rawViewEnd;
@@ -1764,8 +1764,8 @@ bool SharedCache::LoadImageWithInstallName(std::string installName)
 		auto rawViewEnd = m_dscView->GetParentView()->GetEnd();
 
 		auto buff = reader.ReadBuffer(region.start, region.size);
-		m_dscView->GetParentView()->GetParentView()->WriteBuffer(rawViewEnd, *buff);
-		m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
+		m_dscView->GetParentView()->GetParentView()->WriteBuffer(rawViewEnd, buff);
+		m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
 
 		region.loaded = true;
 		region.rawViewOffsetIfLoaded = rawViewEnd;
@@ -1774,7 +1774,7 @@ bool SharedCache::LoadImageWithInstallName(std::string installName)
 
 		m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, region.size, rawViewEnd, region.size, region.flags);
 		m_dscView->AddUserSegment(region.start, region.size, rawViewEnd, region.size, region.flags);
-		m_dscView->WriteBuffer(region.start, *buff);
+		m_dscView->WriteBuffer(region.start, buff);
 
 		regionsToLoad.push_back(&region);
 	}
@@ -2530,7 +2530,7 @@ void SharedCache::InitializeHeader(
 
 		while (i < header.functionStarts.funcsize)
 		{
-			curOffset = readLEB128(*funcStarts, header.functionStarts.funcsize, i);
+			curOffset = readLEB128(funcStarts, header.functionStarts.funcsize, i);
 			bool addFunction = false;
 			for (const auto& region : regionsToLoad)
 			{
@@ -2569,7 +2569,7 @@ void SharedCache::InitializeHeader(
 			if (sym.n_strx >= header.symtab.strsize || ((sym.n_type & N_TYPE) == N_INDR))
 				continue;
 
-			std::string symbol((char*)strtab->GetDataAt(sym.n_strx));
+			std::string symbol((char*)strtab.GetDataAt(sym.n_strx));
 			// BNLogError("%s: 0x%llx", symbol.c_str(), sym.n_value);
 			if (symbol == "<redacted>")
 				continue;
@@ -2769,8 +2769,8 @@ std::vector<Ref<Symbol>> SharedCache::ParseExportTrie(std::shared_ptr<MMappedFil
 
 		std::vector<ExportNode> nodes;
 
-		DataBuffer* buffer = reader->ReadBuffer(header.exportTrie.dataoff, header.exportTrie.datasize);
-		ReadExportNode(symbols, header, *buffer, header.textBase, "", 0, header.exportTrie.datasize);
+		DataBuffer buffer = reader->ReadBuffer(header.exportTrie.dataoff, header.exportTrie.datasize);
+		ReadExportNode(symbols, header, buffer, header.textBase, "", 0, header.exportTrie.datasize);
 	}
 	catch (std::exception& e)
 	{
diff --git a/view/sharedcache/core/VM.cpp b/view/sharedcache/core/VM.cpp
index 2e0a39c5..c351a146 100644
--- a/view/sharedcache/core/VM.cpp
+++ b/view/sharedcache/core/VM.cpp
@@ -453,16 +453,14 @@ int64_t MMappedFileAccessor::ReadLong(size_t address)
 	return ((int64_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
 }
 
-BinaryNinja::DataBuffer* MMappedFileAccessor::ReadBuffer(size_t address, size_t length)
+BinaryNinja::DataBuffer MMappedFileAccessor::ReadBuffer(size_t address, size_t length)
 {
 	if (address > m_mmap.len)
 		throw MappingReadException();
 	if (address + length > m_mmap.len)
 		throw MappingReadException();
 	void* data = (void*)(&(((uint8_t*)m_mmap._mmap)[address]));
-	void* dataCopy = malloc(length);
-	memcpy(dataCopy, data, length);
-	return new BinaryNinja::DataBuffer(dataCopy, length);
+	return BinaryNinja::DataBuffer(data, length);
 }
 
 void MMappedFileAccessor::Read(void* dest, size_t address, size_t length)
@@ -656,7 +654,7 @@ int64_t VM::ReadLong(size_t address)
 	return mapping.first.fileAccessor->lock()->ReadLong(mapping.second);
 }
 
-BinaryNinja::DataBuffer* VM::ReadBuffer(size_t addr, size_t length)
+BinaryNinja::DataBuffer VM::ReadBuffer(size_t addr, size_t length)
 {
 	auto mapping = MappingAtAddress(addr);
 	return mapping.first.fileAccessor->lock()->ReadBuffer(mapping.second, length);
@@ -767,14 +765,14 @@ size_t VMReader::ReadPointer()
 	return 0;
 }
 
-BinaryNinja::DataBuffer* VMReader::ReadBuffer(size_t length)
+BinaryNinja::DataBuffer VMReader::ReadBuffer(size_t length)
 {
 	auto mapping = m_vm->MappingAtAddress(m_cursor);
 	m_cursor += length;
 	return mapping.first.fileAccessor->lock()->ReadBuffer(mapping.second, length);
 }
 
-BinaryNinja::DataBuffer* VMReader::ReadBuffer(size_t addr, size_t length)
+BinaryNinja::DataBuffer VMReader::ReadBuffer(size_t addr, size_t length)
 {
 	auto mapping = m_vm->MappingAtAddress(addr);
 	m_cursor = addr + length;
diff --git a/view/sharedcache/core/VM.h b/view/sharedcache/core/VM.h
index b000266b..b8e5c59b 100644
--- a/view/sharedcache/core/VM.h
+++ b/view/sharedcache/core/VM.h
@@ -172,7 +172,7 @@ public:
 
     int64_t ReadLong(size_t address);
 
-    BinaryNinja::DataBuffer *ReadBuffer(size_t addr, size_t length);
+    BinaryNinja::DataBuffer ReadBuffer(size_t addr, size_t length);
 
     void Read(void *dest, size_t addr, size_t length);
 };
@@ -252,7 +252,7 @@ public:
 
     int64_t ReadLong(size_t address);
 
-    BinaryNinja::DataBuffer *ReadBuffer(size_t addr, size_t length);
+    BinaryNinja::DataBuffer ReadBuffer(size_t addr, size_t length);
 
     void Read(void *dest, size_t addr, size_t length);
 };
@@ -320,9 +320,9 @@ public:
 
     size_t ReadPointer(size_t address);
 
-    BinaryNinja::DataBuffer *ReadBuffer(size_t length);
+    BinaryNinja::DataBuffer ReadBuffer(size_t length);
 
-    BinaryNinja::DataBuffer *ReadBuffer(size_t addr, size_t length);
+    BinaryNinja::DataBuffer ReadBuffer(size_t addr, size_t length);
 
     void Read(void *dest, size_t length);