[SharedCache] Apply .symbols file information when applying an image

This improves symbol recovery drastically on newer shared caches

Related PR: https://github.com/Vector35/binaryninja-api/pull/6210

8 files changed, 93 insertions, 46 deletions

diff --git a/view/sharedcache/core/MachO.cpp b/view/sharedcache/core/MachO.cpp
index e1ec9229..68081710 100644
--- a/view/sharedcache/core/MachO.cpp
+++ b/view/sharedcache/core/MachO.cpp
@@ -457,32 +457,19 @@ std::optional<SharedCacheMachOHeader> SharedCacheMachOHeader::ParseHeaderForAddr
 	return header;
 }
 
-// TODO: Support reading from .symbols file.
-// TODO: Replace view with address size?
-std::vector<CacheSymbol> SharedCacheMachOHeader::ReadSymbolTable(BinaryView& view, VirtualMemory& vm) const
+std::vector<CacheSymbol> SharedCacheMachOHeader::ReadSymbolTable(VirtualMemory& vm, const TableInfo &symbolInfo, const TableInfo &stringInfo) const
 {
-	auto addressSize = view.GetAddressSize();
-	// NOTE: The symbol table will exist within the link edit segment, the table offsets are relative to the file not
-	// the linkedit segment.
-	uint64_t symbolsAddress = GetLinkEditFileBase() + symtab.symoff;
-	uint64_t stringsAddress = GetLinkEditFileBase() + symtab.stroff;
-
-	// TODO: This needs to be passed in as an optional argument.
-	// TODO: Sometimes symbol tables are shared and we have to offset into the table for a specific header.
-	// TODO: The "shared" symbol tables are stored in .symbols files.
-	int nlistStartIndex = 0;
-
 	std::vector<CacheSymbol> symbolList;
-	for (uint64_t i = 0; i < symtab.nsyms; i++)
+	// TODO: This assumes that 95% (or more) are going to be added.
+	symbolList.reserve(symbolInfo.entries);
+	for (uint64_t entryIndex = 0; entryIndex < symbolInfo.entries; entryIndex++)
 	{
-		uint64_t entryIndex = (nlistStartIndex + i);
-
 		nlist_64 nlist = {};
-		if (addressSize == 4)
+		if (vm.GetAddressSize() == 4)
 		{
 			// 32-bit DSC
 			struct nlist nlist32 = {};
-			vm.Read(&nlist, symbolsAddress + (entryIndex * sizeof(nlist32)), sizeof(nlist32));
+			vm.Read(&nlist, symbolInfo.address + (entryIndex * sizeof(nlist32)), sizeof(nlist32));
 			nlist.n_strx = nlist32.n_strx;
 			nlist.n_type = nlist32.n_type;
 			nlist.n_sect = nlist32.n_sect;
@@ -492,24 +479,24 @@ std::vector<CacheSymbol> SharedCacheMachOHeader::ReadSymbolTable(BinaryView& vie
 		else
 		{
 			// 64-bit DSC
-			vm.Read(&nlist, symbolsAddress + (entryIndex * sizeof(nlist)), sizeof(nlist));
+			vm.Read(&nlist, symbolInfo.address + (entryIndex * sizeof(nlist)), sizeof(nlist));
 		}
 
 		auto symbolAddress = nlist.n_value;
 		if (((nlist.n_type & N_TYPE) == N_INDR) || symbolAddress == 0)
 			continue;
 
-		if (nlist.n_strx >= symtab.strsize)
+		if (nlist.n_strx >= stringInfo.entries)
 		{
 			// TODO: where logger?
 			LogError(
 				"Symbol entry at index %llu has a string offset of %u which is outside the strings buffer of size %u "
 			    "for symbol table %x",
-				entryIndex, nlist.n_strx, symtab.strsize, symtab.stroff);
+				entryIndex, nlist.n_strx, stringInfo.address, stringInfo.entries);
 			continue;
 		}
 
-		std::string symbolName = vm.ReadCString(stringsAddress + nlist.n_strx);
+		std::string symbolName = vm.ReadCString(stringInfo.address + nlist.n_strx);
 		if (symbolName == "<redacted>")
 			continue;
 
diff --git a/view/sharedcache/core/MachO.h b/view/sharedcache/core/MachO.h
index 8508edbf..d3a720a9 100644
--- a/view/sharedcache/core/MachO.h
+++ b/view/sharedcache/core/MachO.h
@@ -7,6 +7,15 @@
 
 struct CacheSymbol;
 
+// Used when reading symbol/string table info.
+struct TableInfo
+{
+	// VM address where the reading will begin.
+	uint64_t address;
+	// Number of entries in the table.
+	uint32_t entries;
+};
+
 struct SharedCacheMachOHeader
 {
 	uint64_t textBase = 0;
@@ -61,8 +70,7 @@ struct SharedCacheMachOHeader
 	static std::optional<SharedCacheMachOHeader> ParseHeaderForAddress(
 		std::shared_ptr<VirtualMemory> vm, uint64_t address, const std::string& imagePath);
 
-	// TODO: Replace view with address size?
-	std::vector<CacheSymbol> ReadSymbolTable(BinaryNinja::BinaryView& view, VirtualMemory& vm) const;
+	std::vector<CacheSymbol> ReadSymbolTable(VirtualMemory& vm, const TableInfo &symbolInfo, const TableInfo &stringInfo) const;
 
 	bool AddExportTerminalSymbol(
 		std::vector<CacheSymbol>& symbols, const std::string& symbolName, const uint8_t* current,
diff --git a/view/sharedcache/core/MachOProcessor.cpp b/view/sharedcache/core/MachOProcessor.cpp
index fdb56516..c6e2c288 100644
--- a/view/sharedcache/core/MachOProcessor.cpp
+++ b/view/sharedcache/core/MachOProcessor.cpp
@@ -17,7 +17,7 @@ SharedCacheMachOProcessor::SharedCacheMachOProcessor(Ref<BinaryView> view, std::
 	}
 }
 
-void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
+void SharedCacheMachOProcessor::ApplyHeader(const SharedCache& cache, SharedCacheMachOHeader& header)
 {
 	auto typeLibraryFromName = [&](const std::string& name) -> Ref<TypeLibrary> {
 		// Check to see if we have already loaded the type library.
@@ -32,13 +32,16 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 
 	// Add a section for the header itself.
 	std::string headerSection = fmt::format("{}::__macho_header", header.identifierPrefix);
-	// TODO: Support mach_header (non 64bit)
-	uint64_t headerSectionSize = sizeof(mach_header_64) + header.ident.sizeofcmds;
+	uint64_t machHeaderSize = m_vm->GetAddressSize() == 8 ? sizeof(mach_header_64) : sizeof(mach_header);
+	uint64_t headerSectionSize = machHeaderSize + header.ident.sizeofcmds;
 	m_view->AddUserSection(headerSection, header.textBase, headerSectionSize, ReadOnlyDataSectionSemantics);
 
 	ApplyHeaderSections(header);
 	ApplyHeaderDataVariables(header);
 
+	// Pull the available type library for the image we are loading, so we can apply known types.
+	auto typeLib = typeLibraryFromName(header.installName);
+
 	if (header.linkeditPresent && m_vm->IsAddressMapped(header.linkeditSegment.vmaddr))
 	{
 		if (m_applyFunctions && header.functionStartsPresent)
@@ -49,18 +52,16 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 				m_view->AddFunctionForAnalysis(targetPlatform, func, false);
 		}
 
-		// Pull the available type library for the image we are loading, so we can apply known types.
-		auto typeLib = typeLibraryFromName(header.installName);
 		m_view->BeginBulkModifySymbols();
 
-		// TODO: Why does this need to only happen in linkeditSegment?
 		// Apply symbols from symbol table.
 		if (header.symtab.symoff != 0)
 		{
-			// Mach-O View symtab processing with
-			// a ton of stuff cut out so it can work
 			// NOTE: This table is read relative to the link edit segment file base.
-			const auto symbols = header.ReadSymbolTable(*m_view, *m_vm);
+			// NOTE: This does not handle the shared .symbols cache entry symbols, that is the responsibility of the caller.
+			TableInfo symbolInfo = { header.GetLinkEditFileBase() + header.symtab.symoff, header.symtab.nsyms };
+			TableInfo stringInfo = { header.GetLinkEditFileBase() + header.symtab.stroff, header.symtab.strsize };
+			const auto symbols = header.ReadSymbolTable(*m_vm, symbolInfo, stringInfo);
 			for (const auto& sym : symbols)
 			{
 				auto [symbol, symbolType] = sym.GetBNSymbolAndType(*m_view);
@@ -72,7 +73,6 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 		if (header.exportTriePresent)
 		{
 			// NOTE: This table is read relative to the link edit segment file base.
-			// TODO: Remove this and use the m_symbols in the cache?
 			const auto exportSymbols = header.ReadExportSymbolTrie(*m_vm);
 			for (const auto& sym : exportSymbols)
 			{
@@ -82,6 +82,53 @@ void SharedCacheMachOProcessor::ApplyHeader(SharedCacheMachOHeader& header)
 		}
 		m_view->EndBulkModifySymbols();
 	}
+
+	// Apply symbols from the .symbols cache files.
+	for (const auto &entry: cache.GetEntries())
+	{
+		// NOTE: We check addr size as we only support 64bit .symbols files currently.
+		if (entry.GetType() != CacheEntryType::Symbols && m_vm->GetAddressSize() == 8)
+			continue;
+		const auto& entryHeader = entry.GetHeader();
+
+		// This is where we get the symbol and string table information from in the .symbols file.
+		dyld_cache_local_symbols_info localSymbolsInfo = {};
+		auto localSymbolsInfoAddr = entry.GetMappedAddress(entryHeader.localSymbolsOffset);
+		if (!localSymbolsInfoAddr.has_value())
+			continue;
+		m_vm->Read(&localSymbolsInfo, *localSymbolsInfoAddr, sizeof(dyld_cache_local_symbols_info));
+
+		// Read each symbols entry, looking for the current image entry.
+		uint64_t localEntriesAddr = *localSymbolsInfoAddr + localSymbolsInfo.entriesOffset;
+		uint64_t localSymbolsAddr = *localSymbolsInfoAddr + localSymbolsInfo.nlistOffset;
+		uint64_t localStringsAddr = *localSymbolsInfoAddr + localSymbolsInfo.stringsOffset;
+
+		dyld_cache_local_symbols_entry_64 localSymbolsEntry = {};
+		for (uint32_t i = 0; i < localSymbolsInfo.entriesCount; i++)
+		{
+			m_vm->Read(&localSymbolsEntry, localEntriesAddr + i * sizeof(dyld_cache_local_symbols_entry_64),
+			           sizeof(dyld_cache_local_symbols_entry_64));
+			// The dylibOffset is the offset from the cache base address to the image header.
+			const auto imageAddr = cache.GetBaseAddress() + localSymbolsEntry.dylibOffset;
+			if (imageAddr == header.textBase)
+			{
+				// We have found the entry to read!
+				// TODO: Support 32bit nlist
+				uint64_t symbolTableStart = localSymbolsAddr + (localSymbolsEntry.nlistStartIndex * sizeof(nlist_64));
+				TableInfo symbolInfo = {symbolTableStart, localSymbolsEntry.nlistCount};
+				TableInfo stringInfo = {localStringsAddr, localSymbolsInfo.stringsSize};
+				m_view->BeginBulkModifySymbols();
+				const auto symbols = header.ReadSymbolTable(*m_vm, symbolInfo, stringInfo);
+				for (const auto &sym: symbols)
+				{
+					auto [symbol, symbolType] = sym.GetBNSymbolAndType(*m_view);
+					ApplySymbol(m_view, typeLib, symbol, symbolType);
+				}
+				m_view->EndBulkModifySymbols();
+				break;
+			}
+		}
+	}
 }
 
 uint64_t SharedCacheMachOProcessor::ApplyHeaderSections(SharedCacheMachOHeader& header)
diff --git a/view/sharedcache/core/MachOProcessor.h b/view/sharedcache/core/MachOProcessor.h
index 0564a831..4b014f4e 100644
--- a/view/sharedcache/core/MachOProcessor.h
+++ b/view/sharedcache/core/MachOProcessor.h
@@ -1,5 +1,6 @@
 #pragma once
 #include "MachO.h"
+#include "SharedCache.h"
 
 // Process `SharedCacheMachOHeader`.
 class SharedCacheMachOProcessor
@@ -15,7 +16,7 @@ public:
 		BinaryNinja::Ref<BinaryNinja::BinaryView> view, std::shared_ptr<VirtualMemory> vm);
 
 	// Initialize header information such as sections and symbols.
-	void ApplyHeader(SharedCacheMachOHeader& header);
+	void ApplyHeader(const SharedCache& cache, SharedCacheMachOHeader& header);
 
 	uint64_t ApplyHeaderSections(SharedCacheMachOHeader& header);
 
diff --git a/view/sharedcache/core/SharedCache.cpp b/view/sharedcache/core/SharedCache.cpp
index 717ec6aa..dced1660 100644
--- a/view/sharedcache/core/SharedCache.cpp
+++ b/view/sharedcache/core/SharedCache.cpp
@@ -80,10 +80,14 @@ std::optional<CacheEntry> CacheEntry::FromFile(const std::string& filePath, cons
 		// We found a single dyld data cache entry file. Mark it as such!
 		type = CacheEntryType::DyldData;
 	}
-	else if (fileName.find(".symbols") != std::string::npos)
+	else if (fileName.find(".symbols") != std::string::npos && mappings.size() == 1)
 	{
 		// We found a single symbols cache entry file. Mark it as such!
 		type = CacheEntryType::Symbols;
+		// Adjust the mapping for the symbol file, they seem to be only for the header.
+		// If we do not adjust the mapping than we will not be able to read the symbol table through the virtual memory.
+		mappings[0].fileOffset = 0;
+		mappings[0].size = file->Length();
 	}
 	else if (mappings.size() == 1 && header.imagesCountOld == 0 && header.imagesCount == 0
 		&& header.imagesTextOffset == 0)
@@ -154,8 +158,7 @@ std::optional<uint64_t> CacheEntry::GetMappedAddress(uint64_t fileOffset) const
 
 SharedCache::SharedCache(uint64_t addressSize)
 {
-	m_addressSize = addressSize;
-	m_vm = std::make_shared<VirtualMemory>();
+	m_vm = std::make_shared<VirtualMemory>(addressSize);
 	m_namedSymMutex = std::make_unique<std::shared_mutex>();
 }
 
@@ -321,7 +324,7 @@ void SharedCache::ProcessEntryRegions(const CacheEntry& entry)
 	// Collect pool addresses as non image memory regions.
 	for (size_t i = 0; i < entryHeader.branchPoolsCount; i++)
 	{
-		auto branchPoolAddr = entryHeader.branchPoolsOffset + (i * m_addressSize);
+		auto branchPoolAddr = entryHeader.branchPoolsOffset + (i * m_vm->GetAddressSize());
 		auto header = SharedCacheMachOHeader::ParseHeaderForAddress(
 			m_vm, branchPoolAddr, "dyld_shared_cache_branch_islands_" + std::to_string(i));
 		// Stop processing branch pools if a header fails to parse.
diff --git a/view/sharedcache/core/SharedCache.h b/view/sharedcache/core/SharedCache.h
index 8aea0362..c3e0fb80 100644
--- a/view/sharedcache/core/SharedCache.h
+++ b/view/sharedcache/core/SharedCache.h
@@ -173,7 +173,7 @@ typedef uint32_t CacheEntryId;
 // once every time the database is open.
 class SharedCache
 {
-	uint64_t m_addressSize = 8;
+	// Calculated within `AddEntry`, this indicates where the shared cache image is based at.
 	uint64_t m_baseAddress = 0;
 	// TODO: Figure out when to lock the mutex on this shit lmfao
 	// The shared cache can own the virtual memory, this is fine...
diff --git a/view/sharedcache/core/SharedCacheController.cpp b/view/sharedcache/core/SharedCacheController.cpp
index 8420d358..b6dfdb47 100644
--- a/view/sharedcache/core/SharedCacheController.cpp
+++ b/view/sharedcache/core/SharedCacheController.cpp
@@ -1,10 +1,6 @@
 #include "SharedCacheController.h"
-
-#include <utility>
-
 #include "MachOProcessor.h"
 #include "ObjC.h"
-#include "SlideInfo.h"
 
 using namespace BinaryNinja;
 using namespace BinaryNinja::DSC;
@@ -211,7 +207,7 @@ bool SharedCacheController::ApplyImage(BinaryView& view, const CacheImage& image
 		// analyzed functions as updated.
 		auto prevDisabledState = view.GetFunctionAnalysisUpdateDisabled();
 		view.SetFunctionAnalysisUpdateDisabled(true);
-		machoProcessor.ApplyHeader(*image.header);
+		machoProcessor.ApplyHeader(GetCache(), *image.header);
 		view.SetFunctionAnalysisUpdateDisabled(prevDisabledState);
 
 		// Load objective-c information.
diff --git a/view/sharedcache/core/VirtualMemory.h b/view/sharedcache/core/VirtualMemory.h
index 2edc6ef3..647f4468 100644
--- a/view/sharedcache/core/VirtualMemory.h
+++ b/view/sharedcache/core/VirtualMemory.h
@@ -42,8 +42,13 @@ class VirtualMemory
 {
 	std::shared_mutex m_regionMutex;
 	AddressRangeMap<VirtualMemoryRegion> m_regions;
+	uint64_t m_addressSize = 8;
 
 public:
+	explicit VirtualMemory(uint64_t addressSize = 8) : m_addressSize(addressSize) {}
+
+	uint64_t GetAddressSize() const { return m_addressSize; }
+
 	// At no point do we ever store a strong pointer to a file accessor, that is the job of the `FileAccessorCache`.
 	void MapRegion(WeakFileAccessor fileAccessor, AddressRange mappedRange, uint64_t fileOffset);