From a40db295185fb77be8c7a4f9be03af12284b491a Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 08:32:13 -0300 Subject: [PATCH 01/24] ensure ElfView::ReadStringTable strlen dont overrun --- view/elf/elfview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index bfb5c68b2..8cf5c0259 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -2754,7 +2754,7 @@ string ElfView::ReadStringTable(BinaryReader& reader, const Elf64SectionHeader& } const std::vector& tableCache = itr->second; - return std::string(&tableCache[offset], strlen(tableCache.data() + offset)); + return std::string(&tableCache[offset], strnlen(tableCache.data() + offset, tableCache.size() - offset)); } From b5ea3806616be0b7d1e832b15d0da996289140ec Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 08:38:44 -0300 Subject: [PATCH 02/24] ensure ElfView::ReadStringTable dont read too much data --- view/elf/elfview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 8cf5c0259..6c821f833 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -2734,7 +2734,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 > 0x1000000) return ""; auto itr = m_stringTableCache.find(section.offset); From b629f782b9e57776e4d94e6e3cc8d520c20a8a62 Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:14:12 -0300 Subject: [PATCH 03/24] ensure MachoView::ParseSymbolTable strlen dont overrun --- view/macho/machoview.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index 761391880..fc8bae151 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -3338,7 +3338,8 @@ void MachoView::ParseSymbolTable(BinaryReader& reader, MachOHeader& header, cons if (sym.n_strx >= symtab.strsize || ((sym.n_type & N_TYPE) == N_INDR)) continue; - string symbol((char*)header.stringList.GetDataAt(sym.n_strx)); + const char* symbolName = (const char*)header.stringList.GetDataAt(sym.n_strx); + string symbol(symbolName, strnlen(symbolName, header.stringList.GetLength() - sym.n_strx)); m_symbols.push_back(symbol); //otool ignores symbols that end with ".o", startwith "ltmp" or are "gcc_compiled." so do we if (symbol == "gcc_compiled." || From 8645b416fd4dedfb49a9ce31ec6fbc117bf6a71b Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:17:07 -0300 Subject: [PATCH 04/24] ensure MachoViewType::ParseHeaders have contains enough data --- view/macho/machoview.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index fc8bae151..fde6b0e34 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -3826,8 +3826,13 @@ bool MachoViewType::IsTypeValidForData(BinaryView* data) uint64_t MachoViewType::ParseHeaders(BinaryView* data, uint64_t imageOffset, mach_header_64& ident, Ref* arch, Ref* plat, string& errorMsg) { DataBuffer sig = data->ReadBuffer(imageOffset, 4); + if (sig.GetLength() != 4) + { + errorMsg = "signature too small"; + return 0; + } uint32_t magic = *(uint32_t*)sig.GetData(); - if ((sig.GetLength() != 4) || !(magic == MH_CIGAM || magic == MH_CIGAM_64 || magic == MH_MAGIC || magic == MH_MAGIC_64)) + if (!(magic == MH_CIGAM || magic == MH_CIGAM_64 || magic == MH_MAGIC || magic == MH_MAGIC_64)) { errorMsg = "invalid signature"; return 0; From 19850e9854a68cb77cee2247110d3587ed847d62 Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:20:37 -0300 Subject: [PATCH 05/24] fix MachoViewType::ParseExportTrie end guard --- view/macho/machoview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index fde6b0e34..387720998 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -2862,9 +2862,9 @@ bool MachoView::AddExportTerminalSymbol( void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command exportTrie) { try { - uint32_t endGuard = exportTrie.datasize; DataBuffer buffer = GetParentView() ->ReadBuffer(m_universalImageOffset + exportTrie.dataoff, exportTrie.datasize); + uint32_t endGuard = buffer.GetLength() - 1; struct Node { From 16f0473c8660060687fdd51bb16580a5c595992e Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:27:58 -0300 Subject: [PATCH 06/24] fix PEView::Init divide by zero FPE --- view/pe/peview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/view/pe/peview.cpp b/view/pe/peview.cpp index 6af6283ea..7bb318777 100644 --- a/view/pe/peview.cpp +++ b/view/pe/peview.cpp @@ -783,7 +783,7 @@ bool PEView::Init() else { uint64_t sizeOfImage = opt.sizeOfImage; - if (opt.sizeOfImage % resolvedSectionAlignment) + if (resolvedSectionAlignment && opt.sizeOfImage % resolvedSectionAlignment) sizeOfImage = (opt.sizeOfImage + resolvedSectionAlignment) & ~(resolvedSectionAlignment - 1); uint64_t dataLength = GetParentView()->GetEnd(); dataLength = std::min(std::max((uint64_t)m_sizeOfHeaders, sizeOfImage), dataLength); From f118d72f062f7af9494bc4b0c0bbfb23520ae53f Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:45:13 -0300 Subject: [PATCH 07/24] fix MachoView::Init uncaught exception --- view/macho/machoview.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index 387720998..e781bf472 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -1700,13 +1700,21 @@ bool MachoView::Init() Ref filesetEntryCommandType = Type::StructureType(filesetEntryCommandStruct); m_typeNames.filesetEntryCommandQualName = DefineType(filesetEntryCommandTypeId, filesetEntryCommandName, filesetEntryCommandType); - if (!InitializeHeader(m_header, true, preferredImageBase, preferredImageBaseDesc, platformSetByUser)) - return false; - - for (auto& it : m_subHeaders) + try { - if (!InitializeHeader(it.second, false, it.first, "", platformSetByUser)) + if (!InitializeHeader(m_header, true, preferredImageBase, preferredImageBaseDesc, platformSetByUser)) return false; + + for (auto& it : m_subHeaders) + { + if (!InitializeHeader(it.second, false, it.first, "", platformSetByUser)) + return false; + } + } + catch (std::exception& e) + { + m_logger->LogErrorForExceptionF(e, "MachoView failed to InitializeHeader! {:?}", e.what()); + return false; } std::chrono::steady_clock::time_point endTime = std::chrono::steady_clock::now(); From ffa34034842a25508ee593d8800df4ed9ee8f45b Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 10:53:30 -0300 Subject: [PATCH 08/24] ensure ElfView::ReadStringTable dont read after the offset --- view/elf/elfview.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 6c821f833..218107da0 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -2754,6 +2754,10 @@ string ElfView::ReadStringTable(BinaryReader& reader, const Elf64SectionHeader& } const std::vector& tableCache = itr->second; + 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)); } From 712b40835ea052415b7ff628c94e84fdf9bfb9fb Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 09:34:22 -0300 Subject: [PATCH 09/24] add try catch to BinaryView::InitCallback to help fuzzer --- binaryview.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/binaryview.cpp b/binaryview.cpp index 8dfd5f689..77dcacf84 100644 --- a/binaryview.cpp +++ b/binaryview.cpp @@ -1384,8 +1384,14 @@ BinaryView::BinaryView(BNBinaryView* view) bool BinaryView::InitCallback(void* ctxt) { - CallbackRef view(ctxt); - return view->Init(); + try + { + CallbackRef view(ctxt); + return view->Init(); + } catch (std::exception& e) + { + return false; + } } From 2c4fac65f3d176dedccb7025fac58fedce486aa1 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 12:29:24 -0300 Subject: [PATCH 10/24] limit localMipsSyms in ElfView::Init to sane values --- view/elf/elfview.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 218107da0..9ab7467d0 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -1149,7 +1149,8 @@ bool ElfView::Init() { if (mipsSymValid && (gotStart != 0)) { - for (size_t i = 2; i < localMipsSyms; i++) + auto localMipsSymsFixed = localMipsSyms % 0x1000; + for (size_t i = 2; i < localMipsSymsFixed; i++) { m_gotEntryLocations.emplace(gotStart + i * (m_elf32 ? 4 : 8)); } From 8fd368722d0edcc2a4bb895dd9cb1ff9fea71df9 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 13:47:26 -0300 Subject: [PATCH 11/24] limit nindirectsyms in MachoView::InitializeHeader to sane values --- view/macho/machoview.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index e781bf472..ac447085b 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -2276,9 +2276,10 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_ // Handle indirect symbols if (header.dysymtab.nindirectsyms) { - indirectSymbols.resize(header.dysymtab.nindirectsyms); + auto nindirectsyms_fix = header.dysymtab.nindirectsyms % 0x1000; + indirectSymbols.resize(nindirectsyms_fix); reader.Seek(header.dysymtab.indirectsymoff); - reader.Read(&indirectSymbols[0], header.dysymtab.nindirectsyms * sizeof(uint32_t)); + reader.Read(&indirectSymbols[0], nindirectsyms_fix * sizeof(uint32_t)); } } catch (ReadException&) From 76baa9f63a3a6e534a770cfcae0ee75c4935a1a2 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 14:56:47 -0300 Subject: [PATCH 12/24] limit the stack in MachoView::ParseExportTrie to sane limits --- view/macho/machoview.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index ac447085b..3e9a660cc 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -2884,7 +2884,8 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo stack.reserve(64); stack.push_back({ /* cursor */ 0, /* text */ "" }); - while (!stack.empty()) + uint32_t stack_guard = 0; + while (!stack.empty() && stack_guard++ < 0x1000) { m_logger->LogTraceF("Export Trie: Processing node {:?} with cursor {:#x}", stack.back().text, stack.back().cursor); Node node = std::move(stack.back()); From 02212cc43a23fe20bce7e7494da70cc650e172ae Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 15:53:24 -0300 Subject: [PATCH 13/24] fix the string_view reading out-of-bounds --- view/macho/chained_fixups.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/view/macho/chained_fixups.cpp b/view/macho/chained_fixups.cpp index e71b34b80..d41decd80 100644 --- a/view/macho/chained_fixups.cpp +++ b/view/macho/chained_fixups.cpp @@ -244,8 +244,14 @@ ImportEntry ReadChainedImport32(BinaryReader& reader, std::span symb { dyld_chained_import import; reader.Read(&import, sizeof(import)); + std::string_view view; + if (symbolData.size() > import.name_offset) { + view = std::string_view(&symbolData[import.name_offset]); + } else { + view = std::string_view(); + } return { - std::string_view(&symbolData[import.name_offset]), + view, 0, import.lib_ordinal > 0xF0 ? static_cast(import.lib_ordinal) : static_cast(import.lib_ordinal), (bool)import.weak_import, @@ -435,7 +441,7 @@ void ChainedFixupProcessor::ProcessChainsInSegment(const dyld_chained_starts_in_ bool done = false; while (!done) - { + { uint64_t position = reader.GetOffset(); auto [raw, fixupInfo] = fixupReader(reader); From ac55e6767f588d3183566efb9ddd76b3d9a7aac4 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 15:59:20 -0300 Subject: [PATCH 14/24] fix the ChainedFixupProcessor::ProcessImports missing empty vector guard --- view/macho/chained_fixups.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/view/macho/chained_fixups.cpp b/view/macho/chained_fixups.cpp index d41decd80..f4affb317 100644 --- a/view/macho/chained_fixups.cpp +++ b/view/macho/chained_fixups.cpp @@ -311,6 +311,9 @@ std::vector ChainedFixupProcessor::ProcessImports() const auto header = ReadHeader(reader); uint64_t symbolDataSize = m_fixupsSize - header.symbols_offset; + if (!symbolDataSize) { + return imports; + } m_symbolData.resize(symbolDataSize); m_raw->Read(&m_symbolData[0], OffsetInFixups(header.symbols_offset), symbolDataSize); From 9dbcffba66a0f935ac9aa0d6d26e512b1b0d83a8 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 16:34:33 -0300 Subject: [PATCH 15/24] add loop guard to PEView::Init --- view/pe/peview.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/view/pe/peview.cpp b/view/pe/peview.cpp index 7bb318777..45ef8a4ac 100644 --- a/view/pe/peview.cpp +++ b/view/pe/peview.cpp @@ -1522,7 +1522,8 @@ bool PEView::Init() size_t numImportEntries = 0; vector> libraries; vector> libraryFound; - while (true) + uint32_t guard_1 = 0; + while (guard_1++ < 0x100) { // Read in next directory entry reader.Seek(RVAToFileOffset(dir.virtualAddress + (numImportEntries * 20))); @@ -1614,7 +1615,8 @@ bool PEView::Init() // We should make this second unused data a structure containing this information information // and default it to collapsed...IDA Just doesn't show anything at all m_logger->LogDebug("Name: %s\n", dllName.c_str()); - while (true) + uint32_t guard_2 = 0; + while (guard_2++ < 0x1000) { uint64_t entry; bool isOrdinal; From 1a8cd90358ac9517da86ad64008ef69b7c1613cd Mon Sep 17 00:00:00 2001 From: rbran Date: Tue, 16 Jun 2026 07:55:22 -0300 Subject: [PATCH 16/24] fix macho ImportEntry::ReadChainedImport32 string_view overrun Apply the same name_offset bounds check and strnlen guard already present in ReadChainedImport32 to ReadChainedImportAddend32 and ReadChainedImportAddend64, and extract the shared logic into a SymbolNameAt helper so the three functions share one implementation instead of three slightly different copies. --- view/macho/chained_fixups.cpp | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/view/macho/chained_fixups.cpp b/view/macho/chained_fixups.cpp index f4affb317..5bb1e0be6 100644 --- a/view/macho/chained_fixups.cpp +++ b/view/macho/chained_fixups.cpp @@ -240,18 +240,21 @@ auto FixupReaderForFormat(int format) -> std::pair(*)(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 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 symbolData) { dyld_chained_import import; reader.Read(&import, sizeof(import)); - std::string_view view; - if (symbolData.size() > import.name_offset) { - view = std::string_view(&symbolData[import.name_offset]); - } else { - view = std::string_view(); - } return { - view, + SymbolNameAt(symbolData, import.name_offset), 0, import.lib_ordinal > 0xF0 ? static_cast(import.lib_ordinal) : static_cast(import.lib_ordinal), (bool)import.weak_import, @@ -263,7 +266,7 @@ ImportEntry ReadChainedImportAddend32(BinaryReader& reader, std::span(import.addend), import.lib_ordinal > 0xF0 ? static_cast(import.lib_ordinal) : static_cast(import.lib_ordinal), (bool)import.weak_import, @@ -275,7 +278,7 @@ ImportEntry ReadChainedImportAddend64(BinaryReader& reader, std::span 0xFFF0 ? static_cast(import.lib_ordinal) : static_cast(import.lib_ordinal), (bool)import.weak_import, From ef164b13b272fb539bf1ba04504d6c50610e701c Mon Sep 17 00:00:00 2001 From: rbran Date: Tue, 16 Jun 2026 08:19:50 -0300 Subject: [PATCH 17/24] limit section.size in ElfView::ApplyTypesToParentStringTable to sane values --- view/elf/elfview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 9ab7467d0..40d81c81f 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -2667,7 +2667,7 @@ void ElfView::ApplyTypesToParentStringTable(const Elf64SectionHeader& section, c { m_logger->LogInfo("Found string table of size %p at offset %p", section.size, section.offset); DataBuffer buffer = GetParentView()->ReadBuffer(section.offset, section.size); - if (buffer.GetLength() != section.size) + if (section.size > 0x1000 || buffer.GetLength() != section.size) return; unordered_map> cachedTypes; for (size_t start_address = (offset ? 1 : 0); start_address < section.size; ++start_address) From fd0f58db8f7c967c314a120c969ddf7540e915d7 Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 21 Jun 2026 14:24:00 -0300 Subject: [PATCH 18/24] ensure ElfView::ReadStringTable dont read after the offset by one --- view/elf/elfview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 40d81c81f..99bb11895 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -2755,7 +2755,7 @@ string ElfView::ReadStringTable(BinaryReader& reader, const Elf64SectionHeader& } const std::vector& tableCache = itr->second; - if (offset > tableCache.size()) { + 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 ""; } From c1933fdd6149e971e61ab149f34b7d3b5f4eca49 Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Wed, 1 Jul 2026 13:31:04 -0400 Subject: [PATCH 19/24] Ensure C ABI callbacks in BinaryView handle all exceptions Add try/catch to OnAfterSnapshotDataAppliedCallback and SaveCallback, which previously had no exception handling. Extend InitCallback to catch all exception types and log the error before returning. Co-Authored-By: Claude Sonnet 4.6 --- binaryview.cpp | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/binaryview.cpp b/binaryview.cpp index 77dcacf84..e98a96d7c 100644 --- a/binaryview.cpp +++ b/binaryview.cpp @@ -1388,8 +1388,15 @@ bool BinaryView::InitCallback(void* ctxt) { CallbackRef view(ctxt); return view->Init(); - } catch (std::exception& e) + } + catch (const std::exception& e) + { + LogError("BinaryView::Init failed: %s", e.what()); + return false; + } + catch (...) { + LogError("BinaryView::Init failed with unknown exception"); return false; } } @@ -1397,8 +1404,19 @@ bool BinaryView::InitCallback(void* ctxt) void BinaryView::OnAfterSnapshotDataAppliedCallback(void* ctxt) { - CallbackRef view(ctxt); - view->OnAfterSnapshotDataApplied(); + try + { + CallbackRef view(ctxt); + view->OnAfterSnapshotDataApplied(); + } + catch (const std::exception& e) + { + LogError("BinaryView::OnAfterSnapshotDataApplied failed: %s", e.what()); + } + catch (...) + { + LogError("BinaryView::OnAfterSnapshotDataApplied failed with unknown exception"); + } } @@ -1537,9 +1555,22 @@ size_t BinaryView::GetAddressSizeCallback(void* ctxt) bool BinaryView::SaveCallback(void* ctxt, BNFileAccessor* file) { - CallbackRef view(ctxt); - CoreFileAccessor accessor(file); - return view->PerformSave(&accessor); + try + { + CallbackRef 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; + } } From b243e547c26e489056aa380bb235ea15603ce7be Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Wed, 1 Jul 2026 13:34:28 -0400 Subject: [PATCH 20/24] Improve bounds checking and add file-backed validation in ElfView Replace magic-number loop guards in MIPS GOT processing with segment-derived limits and configurable budgets. Fix integer underflow when localMipsSyms is zero. Correct string table validation to compare against file length rather than virtual address space. Add IsRangeBackedByFile helper that validates every byte in the given range is file-backed, and guard against overflow when a program header's declared file extent is used to bound the GOT entry count. Co-Authored-By: Claude Sonnet 4.6 --- view/elf/elfview.cpp | 80 +++++++++++++++++++++++++++++++++++++++----- view/elf/elfview.h | 2 ++ 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 99bb11895..0a91efb17 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -51,6 +51,28 @@ void BinaryNinja::InitElfViewType() "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] })~"); + settings->RegisterSetting("files.elf.maxMipsLocalGotEntries", + R"({ + "title" : "Maximum MIPS Local GOT Entry Count", + "type" : "number", + "default" : 1048576, + "minValue" : 0, + "maxValue" : 67108864, + "description" : "Maximum number of local GOT entries 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,15 +1171,33 @@ bool ElfView::Init() { if (mipsSymValid && (gotStart != 0)) { - auto localMipsSymsFixed = localMipsSyms % 0x1000; - for (size_t i = 2; i < localMipsSymsFixed; i++) + const uint64_t entrySize = m_elf32 ? 4 : 8; + Ref viewSettings = Settings::Instance(); + const uint64_t entryBudget = viewSettings->Get("files.elf.maxMipsLocalGotEntries", this); + + // 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; @@ -1797,7 +1837,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) @@ -2666,8 +2706,16 @@ 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 viewSettings = Settings::Instance(); + const uint64_t sizeBudget = viewSettings->Get("files.elf.maxStringTableTypeSizeMB", this) * 1024 * 1024; + if (section.size > sizeBudget) + return; DataBuffer buffer = GetParentView()->ReadBuffer(section.offset, section.size); - if (section.size > 0x1000 || buffer.GetLength() != section.size) + if (buffer.GetLength() != section.size) return; unordered_map> cachedTypes; for (size_t start_address = (offset ? 1 : 0); start_address < section.size; ++start_address) @@ -2735,7 +2783,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 || section.size > 0x1000000) + if (offset == 0 || offset >= section.size || section.size > GetParentView()->GetLength()) return ""; auto itr = m_stringTableCache.find(section.offset); @@ -2763,6 +2811,22 @@ string ElfView::ReadStringTable(BinaryReader& reader, const Elf64SectionHeader& } +// 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; +} + + // http://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#FUNC-DES bool ElfView::DerefPpc64Descriptor(BinaryReader& reader, uint64_t addr, uint64_t& result) { diff --git a/view/elf/elfview.h b/view/elf/elfview.h index f81fa832e..a0b56fda4 100644 --- a/view/elf/elfview.h +++ b/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>& 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); From bf065b00d22ec4f40efa14d78d8579ee00def42d Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Wed, 1 Jul 2026 13:34:45 -0400 Subject: [PATCH 21/24] Improve bounds checking and add file-backed validation in PEView Replace magic-number loop guards in import directory and import lookup table parsing with section-backed bounds. Add IsRVARangeBackedByFile helper that validates every byte in the given RVA range resolves to a file-backed offset, rather than only its two endpoints, and guard the import lookup table's section-derived bound against a section header that declares more raw data than the file actually contains. Co-Authored-By: Claude Sonnet 4.6 --- view/pe/peview.cpp | 59 ++++++++++++++++++++++++++++++++++++++++++---- view/pe/peview.h | 1 + 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/view/pe/peview.cpp b/view/pe/peview.cpp index 45ef8a4ac..9541b32e5 100644 --- a/view/pe/peview.cpp +++ b/view/pe/peview.cpp @@ -1522,11 +1522,13 @@ bool PEView::Init() size_t numImportEntries = 0; vector> libraries; vector> libraryFound; - uint32_t guard_1 = 0; - while (guard_1++ < 0x100) + while (true) { // Read in next directory entry - reader.Seek(RVAToFileOffset(dir.virtualAddress + (numImportEntries * 20))); + uint64_t entryRva = dir.virtualAddress + (uint64_t)numImportEntries * 20; + if (!IsRVARangeBackedByFile(entryRva, 20)) + break; + reader.Seek(RVAToFileOffset(entryRva)); PEImportDirectoryEntry importDirEntry; importDirEntry.lookup = reader.Read32(); importDirEntry.timestamp = reader.Read32(); @@ -1615,9 +1617,26 @@ bool PEView::Init() // We should make this second unused data a structure containing this information information // and default it to collapsed...IDA Just doesn't show anything at all m_logger->LogDebug("Name: %s\n", dllName.c_str()); - uint32_t guard_2 = 0; - while (guard_2++ < 0x1000) + const uint32_t importEntrySize = m_is64 ? 8 : 4; + + // Find the section containing entryOffset once so the inner loop needs no per-entry section scan. + uint64_t importSecEnd = 0; + for (const auto& sec : m_sections) { + if ((uint64_t)entryOffset >= sec.virtualAddress && + (uint64_t)entryOffset < (uint64_t)sec.virtualAddress + sec.sizeOfRawData && + sec.virtualSize != 0) + { + // Only trust this section's declared extent up to what the file actually backs. + if ((uint64_t)sec.pointerToRawData + sec.sizeOfRawData <= (uint64_t)GetParentView()->GetLength()) + importSecEnd = (uint64_t)sec.virtualAddress + sec.sizeOfRawData; + break; + } + } + while (true) + { + if ((uint64_t)entryOffset + importEntrySize > importSecEnd) + break; uint64_t entry; bool isOrdinal; if (m_is64) @@ -3453,6 +3472,36 @@ uint32_t PEView::GetRVACharacteristics(uint64_t offset) } +// Returns true only if the entire range [rva, rva+size) maps to file-backed data. +bool PEView::IsRVARangeBackedByFile(uint64_t rva, uint64_t size) const +{ + if (size == 0) + return false; + if (size > UINT64_MAX - rva) + return false; + for (uint64_t offset = rva; offset < rva + size; offset++) + { + bool found = false; + for (const auto& i : m_sections) + { + if (offset >= (uint64_t)i.virtualAddress && + offset < (uint64_t)i.virtualAddress + (uint64_t)i.sizeOfRawData && + i.virtualSize != 0) + { + uint64_t fileOfs = (uint64_t)i.pointerToRawData + (offset - (uint64_t)i.virtualAddress); + if (!GetParentView()->IsOffsetBackedByFile(fileOfs)) + return false; + found = true; + break; + } + } + if (!found) + return false; + } + return true; +} + + string PEView::ReadString(uint64_t rva) { uint64_t offset = RVAToFileOffset(rva); diff --git a/view/pe/peview.h b/view/pe/peview.h index ed79a02d1..13407afaa 100644 --- a/view/pe/peview.h +++ b/view/pe/peview.h @@ -467,6 +467,7 @@ namespace BinaryNinja uint64_t RVAToFileOffset(uint64_t rva, bool except = true); uint32_t GetRVACharacteristics(uint64_t rva); + bool IsRVARangeBackedByFile(uint64_t rva, uint64_t size) const; std::string ReadString(uint64_t rva); uint16_t Read16(uint64_t rva); uint32_t Read32(uint64_t rva); From 7c29ccd785a707b982d7f1ffa58c195211ce8031 Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Wed, 1 Jul 2026 15:47:36 -0400 Subject: [PATCH 22/24] Change MIPS GOT budget setting from entry count to megabytes Mirrors the Linux kernel pattern (fs/binfmt_elf.c) of bounding by total byte size rather than entry count, so the same limit applies equally to 32-bit and 64-bit MIPS ELF files regardless of entry size. The setting files.elf.maxMipsGotMB (default 4 MB) is divided by the per-entry size at load time to derive the effective entry limit. Ref: https://github.com/torvalds/linux/blob/master/fs/binfmt_elf.c Co-Authored-By: Claude Sonnet 4.6 --- view/elf/elfview.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/view/elf/elfview.cpp b/view/elf/elfview.cpp index 0a91efb17..70ea3d69c 100644 --- a/view/elf/elfview.cpp +++ b/view/elf/elfview.cpp @@ -51,14 +51,14 @@ void BinaryNinja::InitElfViewType() "ignore" : ["SettingsProjectScope", "SettingsResourceScope"] })~"); - settings->RegisterSetting("files.elf.maxMipsLocalGotEntries", + settings->RegisterSetting("files.elf.maxMipsGotMB", R"({ - "title" : "Maximum MIPS Local GOT Entry Count", + "title" : "Maximum MIPS GOT Data Size in MB", "type" : "number", - "default" : 1048576, + "default" : 4, "minValue" : 0, - "maxValue" : 67108864, - "description" : "Maximum number of local GOT entries to process in MIPS ELF files", + "maxValue" : 64, + "description" : "Maximum total GOT data size in megabytes to process in MIPS ELF files", "ignore" : ["SettingsProjectScope"] })"); @@ -1173,7 +1173,8 @@ bool ElfView::Init() { const uint64_t entrySize = m_elf32 ? 4 : 8; Ref viewSettings = Settings::Instance(); - const uint64_t entryBudget = viewSettings->Get("files.elf.maxMipsLocalGotEntries", this); + const uint64_t mbBudget = viewSettings->Get("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; From 16a034248364b5ff96f7e7e980af33cfade73e90 Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Thu, 2 Jul 2026 11:18:32 -0400 Subject: [PATCH 23/24] Improve bounds checking and add file-backed validation in MachoView Replace ad-hoc modulo guards with principled bounds derived from file structure where possible. The indirect symbol count is now clamped to the number of entries that physically fit between the table offset and end of file, accounting for the universal (fat) binary slice offset, using the OS-reported file size rather than any field from the binary. Export trie traversal uses the buffer length as its node visit limit, which prevents unbounded traversal including cyclic tries. Rebase and bind opcode entry counts are bounded by a single per-table total derived from the size of the Mach-O slice divided by its pointer size, scaled by a configurable multiplier (loader.macho.maxRebaseBindEntriesMultiplier, default 1.0), shared across all opcodes in one table via emitRebase/ emitBind helpers so every opcode that records an entry consumes the same budget. The endGuard underflow when the export trie buffer is empty, the symbols_offset wraparound in ProcessImports, and a missing catch(...) in MachoView::Init are also fixed. Co-Authored-By: Claude Sonnet 4.6 --- view/macho/chained_fixups.cpp | 5 +- view/macho/machoview.cpp | 261 +++++++++++++++++++--------------- view/macho/machoview.h | 1 + 3 files changed, 152 insertions(+), 115 deletions(-) diff --git a/view/macho/chained_fixups.cpp b/view/macho/chained_fixups.cpp index 5bb1e0be6..76ca15d68 100644 --- a/view/macho/chained_fixups.cpp +++ b/view/macho/chained_fixups.cpp @@ -313,10 +313,9 @@ std::vector ChainedFixupProcessor::ProcessImports() const auto header = ReadHeader(reader); - uint64_t symbolDataSize = m_fixupsSize - header.symbols_offset; - if (!symbolDataSize) { + 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); diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index 3e9a660cc..406696206 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -363,6 +363,16 @@ void BinaryNinja::InitMachoViewType() static MachoViewType type; BinaryViewType::Register(&type); g_machoViewType = &type; + + Settings::Instance()->RegisterSetting("loader.macho.maxRebaseBindEntriesMultiplier", + R"~({ + "title" : "Mach-O Rebase/Bind Table Entry Count Limit Multiplier", + "type" : "number", + "default" : 1.0, + "minValue" : 0.01, + "maxValue" : 10.0, + "description" : "Multiplier applied to the maximum number of rebase/bind entries permitted per table, which is derived from the size of the Mach-O slice divided by its pointer size" + })~"); } MachoView::MachoView(const string& typeName, BinaryView* data, bool parseOnly): BinaryView(typeName, data->GetFile(), data), @@ -1700,21 +1710,13 @@ bool MachoView::Init() Ref filesetEntryCommandType = Type::StructureType(filesetEntryCommandStruct); m_typeNames.filesetEntryCommandQualName = DefineType(filesetEntryCommandTypeId, filesetEntryCommandName, filesetEntryCommandType); - try - { - if (!InitializeHeader(m_header, true, preferredImageBase, preferredImageBaseDesc, platformSetByUser)) - return false; + if (!InitializeHeader(m_header, true, preferredImageBase, preferredImageBaseDesc, platformSetByUser)) + return false; - for (auto& it : m_subHeaders) - { - if (!InitializeHeader(it.second, false, it.first, "", platformSetByUser)) - return false; - } - } - catch (std::exception& e) + for (auto& it : m_subHeaders) { - m_logger->LogErrorForExceptionF(e, "MachoView failed to InitializeHeader! {:?}", e.what()); - return false; + if (!InitializeHeader(it.second, false, it.first, "", platformSetByUser)) + return false; } std::chrono::steady_clock::time_point endTime = std::chrono::steady_clock::now(); @@ -2276,10 +2278,22 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_ // Handle indirect symbols if (header.dysymtab.nindirectsyms) { - auto nindirectsyms_fix = header.dysymtab.nindirectsyms % 0x1000; - indirectSymbols.resize(nindirectsyms_fix); - reader.Seek(header.dysymtab.indirectsymoff); - reader.Read(&indirectSymbols[0], nindirectsyms_fix * sizeof(uint32_t)); + // Clamp to the number of 4-byte entries that fit between indirectsymoff and + // end-of-file; GetParentView()->GetLength() is the OS-reported file size and + // is not derived from any field inside the binary. indirectsymoff is relative + // to the start of this slice (reader has m_universalImageOffset as its virtual + // base), so that offset must be added here to compute the real file position. + const uint64_t readOffset = m_universalImageOffset + header.dysymtab.indirectsymoff; + const uint64_t fileRemaining = (GetParentView()->GetLength() > readOffset) + ? (GetParentView()->GetLength() - readOffset) / sizeof(uint32_t) + : 0; + const uint32_t indirectSymCount = (uint32_t)std::min((uint64_t)header.dysymtab.nindirectsyms, fileRemaining); + if (indirectSymCount) + { + indirectSymbols.resize(indirectSymCount); + reader.Seek(header.dysymtab.indirectsymoff); + reader.Read(&indirectSymbols[0], indirectSymCount * sizeof(uint32_t)); + } } } catch (ReadException&) @@ -2873,7 +2887,9 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo try { DataBuffer buffer = GetParentView() ->ReadBuffer(m_universalImageOffset + exportTrie.dataoff, exportTrie.datasize); - uint32_t endGuard = buffer.GetLength() - 1; + if (buffer.GetLength() == 0) + return; + size_t endIndex = buffer.GetLength() - 1; struct Node { @@ -2884,8 +2900,11 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo stack.reserve(64); stack.push_back({ /* cursor */ 0, /* text */ "" }); - uint32_t stack_guard = 0; - while (!stack.empty() && stack_guard++ < 0x1000) + // Each trie node consumes at least one byte from the buffer, so buffer.GetLength() + // is a sound upper bound on the number of node visits and also prevents + // infinite traversal when the trie contains cycles. + size_t visitCount = 0; + while (!stack.empty() && visitCount++ < buffer.GetLength()) { m_logger->LogTraceF("Export Trie: Processing node {:?} with cursor {:#x}", stack.back().text, stack.back().cursor); Node node = std::move(stack.back()); @@ -2894,7 +2913,7 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo uint64_t cursor = node.cursor; const std::string currentText = std::move(node.text); - if (cursor > endGuard) + if (cursor > endIndex) { m_logger->LogError("Export Trie: Cursor left trie during initial bounds check"); throw ReadException(); @@ -2915,14 +2934,14 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo } localCursor = childOffset; - if (localCursor > endGuard) + if (localCursor > endIndex) { m_logger->LogError("Export Trie: Cursor left trie while moving to child offset"); throw ReadException(); } uint8_t childCount = buffer[localCursor++]; - if (localCursor > endGuard) + if (localCursor > endIndex) { m_logger->LogError("Export Trie: Cursor left trie while reading child count"); throw ReadException(); @@ -2932,18 +2951,18 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo children.reserve(childCount); for (uint8_t i = 0; i < childCount; ++i) { - if (localCursor > endGuard) + if (localCursor > endIndex) { m_logger->LogError("Export Trie: Cursor left trie while reading child count"); throw ReadException(); } std::string childText; - while (localCursor <= endGuard && buffer[localCursor] != 0) { + while (localCursor <= endIndex && buffer[localCursor] != 0) { childText.push_back(buffer[localCursor++]); } localCursor++; // skip the `\0` - if (localCursor > endGuard) + if (localCursor > endIndex) { m_logger->LogError("Export Trie: Cursor left trie while reading child text"); throw ReadException(); @@ -2972,11 +2991,26 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo } } +// Scale the entry limit by the size of this Mach-O slice divided by its pointer size, +// the maximum number of pointer-sized slots the slice can contain. +uint64_t MachoView::GetRebaseBindEntryLimit() +{ + uint64_t sliceSize = (GetParentView()->GetLength() > m_universalImageOffset) + ? (GetParentView()->GetLength() - m_universalImageOffset) + : 0; + uint64_t structuralLimit = sliceSize / m_addressSize; + double entryLimitMultiplier = Settings::Instance()->Get("loader.macho.maxRebaseBindEntriesMultiplier", this); + return (uint64_t)(structuralLimit * entryLimitMultiplier); +} + + void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint32_t tableOffset, uint32_t tableSize) { if (tableSize == 0 || tableOffset == 0) return; + uint64_t remainingIterations = GetRebaseBindEntryLimit(); + std::function segmentActualLoadAddress = [&](uint64_t segmentIndex) { if (segmentIndex >= header.segments.size()) throw ReadException(); @@ -3000,9 +3034,35 @@ void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint uint64_t segmentEndAddress = segmentActualEndAddress(0); uint64_t count; uint64_t skip; - bool done = false; + bool tableDone = false; size_t i = 0; - while ( !done && (i < tableSize)) + + // Records a rebase at the current address and consumes one unit of the entry + // budget. Returns false once the budget is exhausted, without recording anything. + auto emitRebase = [&]() -> bool { + m_logger->LogTraceF("Rebasing address {:#x}", address); + if (address < segmentStartAddress || address >= segmentEndAddress) + { + m_logger->LogError("Rebase address out of segment bounds"); + throw ReadException(); + } + if (remainingIterations == 0) + { + m_logger->LogWarn("Rebase table encodes more entries than the configured limit allows; ignoring the remainder"); + return false; + } + remainingIterations--; + memset(&rebaseRelocation, 0, sizeof(rebaseRelocation)); + rebaseRelocation.nativeType = BINARYNINJA_MANUAL_RELOCATION; + rebaseRelocation.address = address; + rebaseRelocation.size = m_addressSize; + rebaseRelocation.pcRelative = false; + rebaseRelocation.external = false; + header.rebaseRelocations.push_back(rebaseRelocation); + return true; + }; + + while ( !tableDone && (i < tableSize)) { uint8_t opAndIm = table[i]; uint8_t opcode = opAndIm & RebaseOpcodeMask; @@ -3012,7 +3072,7 @@ void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint switch (opcode) { case RebaseOpcodeDone: - done = true; + tableDone = true; break; case RebaseOpcodeSetTypeImmediate: break; @@ -3029,22 +3089,14 @@ void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint address += immediate * m_addressSize; break; case RebaseOpcodeDoRebaseImmediateTimes: - count = immediate; - for (uint64_t j = 0; j < count; ++j) + // immediate is the low 4 bits of the opcode byte; its value is 0-15. + for (uint64_t j = 0; j < immediate; ++j) { - m_logger->LogTraceF("Rebasing address {:#x}", address); - if (address < segmentStartAddress || address >= segmentEndAddress) + if (!emitRebase()) { - m_logger->LogError("Rebase address out of segment bounds"); - throw ReadException(); + tableDone = true; + break; } - memset(&rebaseRelocation, 0, sizeof(rebaseRelocation)); - rebaseRelocation.nativeType = BINARYNINJA_MANUAL_RELOCATION; - rebaseRelocation.address = address; - rebaseRelocation.size = m_addressSize; - rebaseRelocation.pcRelative = false; - rebaseRelocation.external = false; - header.rebaseRelocations.push_back(rebaseRelocation); address += m_addressSize; } break; @@ -3052,56 +3104,32 @@ void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint count = readLEB128(table, tableSize, i); for (uint64_t j = 0; j < count; ++j) { - m_logger->LogTraceF("Rebasing address {:#x}", address); - if (address < segmentStartAddress || address >= segmentEndAddress) + if (!emitRebase()) { - m_logger->LogError("Rebase address out of segment bounds"); - throw ReadException(); + tableDone = true; + break; } - memset(&rebaseRelocation, 0, sizeof(rebaseRelocation)); - rebaseRelocation.nativeType = BINARYNINJA_MANUAL_RELOCATION; - rebaseRelocation.address = address; - rebaseRelocation.size = m_addressSize; - rebaseRelocation.pcRelative = false; - rebaseRelocation.external = false; - header.rebaseRelocations.push_back(rebaseRelocation); address += m_addressSize; } break; case RebaseOpcodeDoRebaseAddAddressUleb: - m_logger->LogTraceF("Rebasing address {:#x}", address); - if (address < segmentStartAddress || address >= segmentEndAddress) + if (!emitRebase()) { - m_logger->LogError("Rebase address out of segment bounds"); - throw ReadException(); + tableDone = true; + break; } - memset(&rebaseRelocation, 0, sizeof(rebaseRelocation)); - rebaseRelocation.nativeType = BINARYNINJA_MANUAL_RELOCATION; - rebaseRelocation.address = address; - rebaseRelocation.size = m_addressSize; - rebaseRelocation.pcRelative = false; - rebaseRelocation.external = false; - header.rebaseRelocations.push_back(rebaseRelocation); address += readLEB128(table, tableSize, i) + m_addressSize; break; case RebaseOpcodeDoRebaseUlebTimesSkippingUleb: count = readLEB128(table, tableSize, i); - skip = readLEB128(table, tableSize, i); + skip = readLEB128(table, tableSize, i); for (uint64_t j = 0; j < count; ++j) { - m_logger->LogTraceF("Rebasing address {:#x}", address); - if (address < segmentStartAddress || address >= segmentEndAddress) + if (!emitRebase()) { - m_logger->LogError("Rebase address out of segment bounds"); - throw ReadException(); + tableDone = true; + break; } - memset(&rebaseRelocation, 0, sizeof(rebaseRelocation)); - rebaseRelocation.nativeType = BINARYNINJA_MANUAL_RELOCATION; - rebaseRelocation.address = address; - rebaseRelocation.size = m_addressSize; - rebaseRelocation.pcRelative = false; - rebaseRelocation.external = false; - header.rebaseRelocations.push_back(rebaseRelocation); address += skip + m_addressSize; } break; @@ -3122,6 +3150,8 @@ void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint void MachoView::ParseDynamicTable(BinaryReader& reader, MachOHeader& header, BNSymbolType incomingType, uint32_t tableOffset, uint32_t tableSize, BNSymbolBinding binding) { + uint64_t remainingIterations = GetRebaseBindEntryLimit(); + try { reader.Seek(tableOffset); auto table = reader.Read(tableSize); @@ -3137,8 +3167,28 @@ void MachoView::ParseDynamicTable(BinaryReader& reader, MachOHeader& header, BNS // uint32_t flags = 0; // uint32_t type = 0; size_t i = 0; - //bool done = false; - while (i < tableSize) + bool tableDone = false; + + // Records a bind at the current address and consumes one unit of the entry + // budget. Returns false once the budget is exhausted, without recording anything. + auto emitBind = [&]() -> bool { + if (remainingIterations == 0) + { + m_logger->LogWarn("Bind table encodes more entries than the configured limit allows; ignoring the remainder"); + return false; + } + remainingIterations--; + memset(&externReloc, 0, sizeof(externReloc)); + externReloc.nativeType = BINARYNINJA_MANUAL_RELOCATION; + externReloc.address = address; + externReloc.size = m_addressSize; + externReloc.pcRelative = false; + externReloc.external = true; + header.bindingRelocations.emplace_back(externReloc, string(name), ordinal); + return true; + }; + + while (i < tableSize && !tableDone) { uint8_t opcode = table[i] & BindOpcodeMask; uint8_t imm = table[i] & BindImmediateMask; @@ -3181,42 +3231,32 @@ void MachoView::ParseDynamicTable(BinaryReader& reader, MachOHeader& header, BNS case BindOpcodeDoBind: if (name == NULL) throw MachoFormatException(); - - memset(&externReloc, 0, sizeof(externReloc)); - externReloc.nativeType = BINARYNINJA_MANUAL_RELOCATION; - externReloc.address = address; - externReloc.size = m_addressSize; - externReloc.pcRelative = false; - externReloc.external = true; - header.bindingRelocations.emplace_back(externReloc, string(name), ordinal); + if (!emitBind()) + { + tableDone = true; + break; + } address += m_addressSize; break; case BindOpcodeDoBindAddAddressULEB: if (name == NULL) throw MachoFormatException(); - - memset(&externReloc, 0, sizeof(externReloc)); - externReloc.nativeType = BINARYNINJA_MANUAL_RELOCATION; - externReloc.address = address; - externReloc.size = m_addressSize; - externReloc.pcRelative = false; - externReloc.external = true; - header.bindingRelocations.emplace_back(externReloc, string(name), ordinal); - + if (!emitBind()) + { + tableDone = true; + break; + } address += m_addressSize; address += readLEB128(table, tableSize, i); break; case BindOpcodeDoBindAddAddressImmediateScaled: if (name == NULL) throw MachoFormatException(); - - memset(&externReloc, 0, sizeof(externReloc)); - externReloc.nativeType = BINARYNINJA_MANUAL_RELOCATION; - externReloc.address = address; - externReloc.size = m_addressSize; - externReloc.pcRelative = false; - externReloc.external = true; - header.bindingRelocations.emplace_back(externReloc, string(name), ordinal); + if (!emitBind()) + { + tableDone = true; + break; + } address += m_addressSize; address += (imm * m_addressSize); break; @@ -3226,17 +3266,14 @@ void MachoView::ParseDynamicTable(BinaryReader& reader, MachOHeader& header, BNS throw MachoFormatException(); uint64_t count = readLEB128(table, tableSize, i); - uint64_t skip = readLEB128(table, tableSize, i); + uint64_t skip = readLEB128(table, tableSize, i); for (; count > 0; count--) { - memset(&externReloc, 0, sizeof(externReloc)); - externReloc.nativeType = BINARYNINJA_MANUAL_RELOCATION; - externReloc.address = address; - externReloc.size = m_addressSize; - externReloc.pcRelative = false; - externReloc.external = true; - header.bindingRelocations.emplace_back(externReloc, string(name), ordinal); - + if (!emitBind()) + { + tableDone = true; + break; + } address += skip + m_addressSize; } break; diff --git a/view/macho/machoview.h b/view/macho/machoview.h index 0e0216f96..b997af750 100644 --- a/view/macho/machoview.h +++ b/view/macho/machoview.h @@ -1514,6 +1514,7 @@ namespace BinaryNinja void ReadExportNode(uint64_t viewStart, DataBuffer& buffer, const std::string& currentText, size_t cursor, uint32_t endGuard); + uint64_t GetRebaseBindEntryLimit(); void ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint32_t tableOffset, uint32_t tableSize); void ParseDynamicTable(BinaryReader& reader, MachOHeader& header, BNSymbolType type, uint32_t tableOffset, uint32_t tableSize, BNSymbolBinding binding); From 871c1a3854f6ed4a51f008f3f6bbedbee5c541bd Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Thu, 2 Jul 2026 11:45:09 -0400 Subject: [PATCH 24/24] Fix potential underflow in MachoView::ParseSymbolTable string bounds check The existing guard compared sym.n_strx against symtab.strsize, but the string list buffer is filled by Read() which may return fewer bytes than requested on a truncated file. When the buffer is shorter than strsize, an n_strx value that passes the strsize check can still exceed the actual buffer length, causing the strnlen length argument to underflow and read past the end of the buffer. Replacing the check with a comparison against stringList.GetLength() ensures the subtraction always stays in bounds. Co-Authored-By: Claude Sonnet 4.6 --- view/macho/machoview.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index 406696206..c3eebc737 100644 --- a/view/macho/machoview.cpp +++ b/view/macho/machoview.cpp @@ -3382,7 +3382,10 @@ void MachoView::ParseSymbolTable(BinaryReader& reader, MachOHeader& header, cons sym.n_value = (m_addressSize == 4) ? reader.Read32() : reader.Read64(); if (sym.n_value) sym.n_value += m_imageBaseAdjustment; - if (sym.n_strx >= symtab.strsize || ((sym.n_type & N_TYPE) == N_INDR)) + // Use GetLength() rather than symtab.strsize because Read() may return + // fewer bytes than requested; checking strsize would allow n_strx to pass + // while still being past the end of the actual buffer. + if (sym.n_strx >= header.stringList.GetLength() || ((sym.n_type & N_TYPE) == N_INDR)) continue; const char* symbolName = (const char*)header.stringList.GetDataAt(sym.n_strx);