Extend rbran's parser robustness fixes with file-backed bounds and configurable limits

binaryview.cpp

@@ -1384,15 +1384,39 @@ BinaryView::BinaryView(BNBinaryView* view)
 
 bool BinaryView::InitCallback(void* ctxt)
 {
-	CallbackRef<BinaryView> view(ctxt);
-	return view->Init();
+	try
+	{
+		CallbackRef<BinaryView> view(ctxt);
+		return view->Init();
+	}
+	catch (const std::exception& e)
+	{
+		LogError("BinaryView::Init failed: %s", e.what());
+		return false;
+	}
+	catch (...)
+	{
+		LogError("BinaryView::Init failed with unknown exception");
+		return false;
+	}
 }
 
 
 void BinaryView::OnAfterSnapshotDataAppliedCallback(void* ctxt)
 {
-	CallbackRef<BinaryView> view(ctxt);
-	view->OnAfterSnapshotDataApplied();
+	try
+	{
+		CallbackRef<BinaryView> view(ctxt);
+		view->OnAfterSnapshotDataApplied();
+	}
+	catch (const std::exception& e)
+	{
+		LogError("BinaryView::OnAfterSnapshotDataApplied failed: %s", e.what());
+	}
+	catch (...)
+	{
+		LogError("BinaryView::OnAfterSnapshotDataApplied failed with unknown exception");
+	}
 }
 
 
@@ -1531,9 +1555,22 @@ size_t BinaryView::GetAddressSizeCallback(void* ctxt)
 
 bool BinaryView::SaveCallback(void* ctxt, BNFileAccessor* file)
 {
-	CallbackRef<BinaryView> view(ctxt);
-	CoreFileAccessor accessor(file);
-	return view->PerformSave(&accessor);
+	try
+	{
+		CallbackRef<BinaryView> view(ctxt);
+		CoreFileAccessor accessor(file);
+		return view->PerformSave(&accessor);
+	}
+	catch (const std::exception& e)
+	{
+		LogError("BinaryView::Save failed: %s", e.what());
+		return false;
+	}
+	catch (...)
+	{
+		LogError("BinaryView::Save failed with unknown exception");
+		return false;
+	}
 }
 
 

view/elf/elfview.cpp

@@ -51,6 +51,28 @@ void BinaryNinja::InitElfViewType()
 		"ignore" : ["SettingsProjectScope", "SettingsResourceScope"]
 		})~");
 
+	settings->RegisterSetting("files.elf.maxMipsGotMB",
+		R"({
+		"title" : "Maximum MIPS GOT Data Size in MB",
+		"type" : "number",
+		"default" : 4,
+		"minValue" : 0,
+		"maxValue" : 64,
+		"description" : "Maximum total GOT data size in megabytes to process in MIPS ELF files",
+		"ignore" : ["SettingsProjectScope"]
+		})");
+
+	settings->RegisterSetting("files.elf.maxStringTableTypeSizeMB",
+		R"~({
+		"title" : "Maximum ELF String Table Size for Type Application (MB)",
+		"type" : "number",
+		"default" : 4,
+		"minValue" : 0,
+		"maxValue" : 256,
+		"description" : "Maximum string table size in megabytes when applying types from string tables",
+		"ignore" : ["SettingsProjectScope"]
+		})~");
+
 }
 
 
@@ -1149,14 +1171,34 @@ bool ElfView::Init()
 	{
 		if (mipsSymValid && (gotStart != 0))
 		{
-			for (size_t i = 2; i < localMipsSyms; i++)
+			const uint64_t entrySize = m_elf32 ? 4 : 8;
+			Ref<Settings> viewSettings = Settings::Instance();
+			const uint64_t mbBudget = viewSettings->Get<uint64_t>("files.elf.maxMipsGotMB", this);
+			const uint64_t entryBudget = (mbBudget * 1024 * 1024) / entrySize;
+
+			// Find the file-backed region containing gotStart once, so the loop needs no per-entry check.
+			uint64_t gotFileEnd = gotStart;
+			for (const auto& hdr : m_programHeaders)
 			{
-				m_gotEntryLocations.emplace(gotStart + i * (m_elf32 ? 4 : 8));
+				if (hdr.type != ELF_PT_LOAD || hdr.fileSize == 0 || hdr.fileSize > UINT64_MAX - hdr.virtualAddress)
+					continue;
+				uint64_t segEnd = hdr.virtualAddress + hdr.fileSize;
+				if (hdr.virtualAddress > gotStart || gotStart >= segEnd)
+					continue;
+				// Only trust this segment's declared extent up to what the file actually backs.
+				if (hdr.fileSize > UINT64_MAX - hdr.offset || hdr.offset + hdr.fileSize > (uint64_t)GetParentView()->GetLength())
+					continue;
+				gotFileEnd = segEnd;
+				break;
 			}
+			const uint64_t maxFromFile = (gotFileEnd > gotStart) ? (gotFileEnd - gotStart) / entrySize : 0;
+			const uint64_t limit = std::min({localMipsSyms, entryBudget, maxFromFile});
+			for (size_t i = 2; i < limit; i++)
+				m_gotEntryLocations.emplace(gotStart + i * entrySize);
 			for (uint64_t i = firstMipsSym; i < (m_auxSymbolTable.size / (m_elf32 ? 16 : 24)); i++)
 			{
-				uint64_t gotEntry = gotStart + ((localMipsSyms + i - firstMipsSym) * (m_elf32 ? 4 : 8));
-				if (!IsValidOffset(gotEntry))
+				uint64_t gotEntry = gotStart + ((localMipsSyms + i - firstMipsSym) * entrySize);
+				if (!IsRangeBackedByFile(gotEntry, entrySize))
 				{
 					m_logger->LogWarn("ELF GOT entry %" PRIx64 " is invalid", gotEntry);
 					break;
@@ -1796,7 +1838,7 @@ bool ElfView::Init()
 		}
 
 		// Perform fixup processing on the local GOT entries if the view is relocatable.
-		if (m_relocatable)
+		if (m_relocatable && localMipsSyms > 0)
 		{
 			uint64_t lastLocalGotEntry = gotStart + (localMipsSyms - 1) * (m_elf32 ? 4 : 8);
 			for (auto gotEntry : m_gotEntryLocations)
@@ -2665,6 +2707,14 @@ void ElfView::DefineElfSymbol(BNSymbolType type, const string& incomingName, uin
 void ElfView::ApplyTypesToParentStringTable(const Elf64SectionHeader& section, const bool offset)
 {
 	m_logger->LogInfo("Found string table of size %p at offset %p", section.size, section.offset);
+	if (section.size == 0 ||
+	    section.size > UINT64_MAX - section.offset ||
+	    section.offset + section.size > GetParentView()->GetLength())
+		return;
+	Ref<Settings> viewSettings = Settings::Instance();
+	const uint64_t sizeBudget = viewSettings->Get<uint64_t>("files.elf.maxStringTableTypeSizeMB", this) * 1024 * 1024;
+	if (section.size > sizeBudget)
+		return;
 	DataBuffer buffer = GetParentView()->ReadBuffer(section.offset, section.size);
 	if (buffer.GetLength() != section.size)
 		return;
@@ -2734,7 +2784,7 @@ void ElfView::ApplyTypesToStringTable(const Elf64SectionHeader& section, const i
 
 string ElfView::ReadStringTable(BinaryReader& reader, const Elf64SectionHeader& section, uint64_t offset)
 {
-	if (offset == 0 || offset > section.size)
+	if (offset == 0 || offset >= section.size || section.size > GetParentView()->GetLength())
 		return "";
 
 	auto itr = m_stringTableCache.find(section.offset);
@@ -2754,7 +2804,27 @@ string ElfView::ReadStringTable(BinaryReader& reader, const Elf64SectionHeader&
 	}
 
 	const std::vector<char>& tableCache = itr->second;
-	return std::string(&tableCache[offset], strlen(tableCache.data() + offset));
+	if (offset >= tableCache.size()) {
+		m_logger->LogError("Unable to read string from table cache offset: 0x%" PRIx64 " size: 0x%" PRIx64, section.offset, section.size);
+		return "";
+	}
+	return std::string(&tableCache[offset], strnlen(tableCache.data() + offset, tableCache.size() - offset));
+}
+
+
+// Returns true only if the entire range [start, start+size) is backed by file data.
+bool ElfView::IsRangeBackedByFile(uint64_t start, uint64_t size) const
+{
+	if (size == 0)
+		return false;
+	if (size > UINT64_MAX - start)
+		return false;
+	for (uint64_t offset = start; offset < start + size; offset++)
+	{
+		if (!IsOffsetBackedByFile(offset))
+			return false;
+	}
+	return true;
 }
 
 

view/elf/elfview.h

@@ -527,6 +527,8 @@ namespace BinaryNinja
 		void DefineElfSymbol(BNSymbolType type, const std::string& name, uint64_t addr, bool gotEntry,
 			BNSymbolBinding binding, size_t size = 0, const Confidence<Ref<Type>>& typeObj = nullptr);
 
+		bool IsRangeBackedByFile(uint64_t start, uint64_t size) const;
+
 		void ApplyTypesToParentStringTable(const Elf64SectionHeader& section, const bool offset = true);
 		void ApplyTypesToStringTable(const Elf64SectionHeader& section, const int64_t imageBaseAdjustment, const bool offset = true);
 		std::string ReadStringTable(BinaryReader& view, const Elf64SectionHeader& section, uint64_t offset);

view/macho/chained_fixups.cpp

@@ -240,12 +240,21 @@ auto FixupReaderForFormat(int format) -> std::pair<uint64_t, FixupInfo>(*)(Binar
 	throw std::invalid_argument("Unknown chained pointer format: " + std::to_string(format));
 }
 
+// Returns the NUL-terminated string starting at `offset` within `symbolData`, or an
+// empty view if `offset` does not fall within `symbolData`.
+std::string_view SymbolNameAt(std::span<const char> symbolData, uint32_t offset)
+{
+	if (symbolData.size() <= offset)
+		return std::string_view();
+	return std::string_view(&symbolData[offset], strnlen(&symbolData[offset], symbolData.size() - offset));
+}
+
 ImportEntry ReadChainedImport32(BinaryReader& reader, std::span<const char> symbolData)
 {
 	dyld_chained_import import;
 	reader.Read(&import, sizeof(import));
 	return {
-		std::string_view(&symbolData[import.name_offset]),
+		SymbolNameAt(symbolData, import.name_offset),
 		0,
 		import.lib_ordinal > 0xF0 ? static_cast<int8_t>(import.lib_ordinal) : static_cast<int32_t>(import.lib_ordinal),
 		(bool)import.weak_import,
@@ -257,7 +266,7 @@ ImportEntry ReadChainedImportAddend32(BinaryReader& reader, std::span<const char
 	dyld_chained_import_addend import;
 	reader.Read(&import, sizeof(import));
 	return {
-		std::string_view(&symbolData[import.name_offset]),
+		SymbolNameAt(symbolData, import.name_offset),
 		static_cast<uint32_t>(import.addend),
 		import.lib_ordinal > 0xF0 ? static_cast<int8_t>(import.lib_ordinal) : static_cast<int32_t>(import.lib_ordinal),
 		(bool)import.weak_import,
@@ -269,7 +278,7 @@ ImportEntry ReadChainedImportAddend64(BinaryReader& reader, std::span<const char
 	dyld_chained_import_addend64 import;
 	reader.Read(&import, sizeof(import));
 	return {
-		std::string_view(&symbolData[import.name_offset]),
+		SymbolNameAt(symbolData, import.name_offset),
 		import.addend,
 		import.lib_ordinal > 0xFFF0 ? static_cast<int16_t>(import.lib_ordinal) : static_cast<int32_t>(import.lib_ordinal),
 		(bool)import.weak_import,
@@ -304,6 +313,8 @@ std::vector<ImportEntry> ChainedFixupProcessor::ProcessImports() const
 
 	auto header = ReadHeader(reader);
 
+	if (header.symbols_offset >= m_fixupsSize)
+		return imports;
 	uint64_t symbolDataSize = m_fixupsSize - header.symbols_offset;
 	m_symbolData.resize(symbolDataSize);
 	m_raw->Read(&m_symbolData[0], OffsetInFixups(header.symbols_offset), symbolDataSize);
@@ -435,7 +446,7 @@ void ChainedFixupProcessor::ProcessChainsInSegment(const dyld_chained_starts_in_
 
 		bool done = false;
 		while (!done)
-		{ 
+		{
 			uint64_t position = reader.GetOffset();
 			auto [raw, fixupInfo] = fixupReader(reader);