From e789a4b56c1b1e025045ef5f595b66df3f3fa68d Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 09:34:22 -0300 Subject: [PATCH 01/13] 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 fa1f36dc1b662abb09a9b86356dc07a61f71230a Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Wed, 1 Jul 2026 13:31:04 -0400 Subject: [PATCH 02/13] 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 479d5b95652e47f49f5fd685bf1006b45925e4d1 Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:14:12 -0300 Subject: [PATCH 03/13] 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 952c15d476e029acb5e625457a9ee9117dc3e2ff Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:17:07 -0300 Subject: [PATCH 04/13] 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 588e0510fc02a970e38fa25cd1e0c9d0f434158f Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:20:37 -0300 Subject: [PATCH 05/13] 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 1cdfaa97bad6233ac0fd30e2797e3c34be6214b8 Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:45:13 -0300 Subject: [PATCH 06/13] 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 eea05ebdfc6edf4ecff46cec152c00a1bd09ab6b Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 13:47:26 -0300 Subject: [PATCH 07/13] 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 00ec5de9e0d4786c9a7939de63164af4d000b616 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 14:56:47 -0300 Subject: [PATCH 08/13] 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 d06c6fa85d6d0f1d598ab9533d31e7be44523b55 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 15:53:24 -0300 Subject: [PATCH 09/13] 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 e38d197f3723ddb3d25316329115f40d59aa1784 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 15:59:20 -0300 Subject: [PATCH 10/13] 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 3d0d31e10f1636fed463dccfe7e39db116ce8f17 Mon Sep 17 00:00:00 2001 From: rbran Date: Tue, 16 Jun 2026 07:55:22 -0300 Subject: [PATCH 11/13] 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 937df03b7701d91f2c95d9a80574e9912252165e Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Thu, 2 Jul 2026 11:18:32 -0400 Subject: [PATCH 12/13] 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 2f6303830ac54132f954b8b95d3748285c683b48 Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Thu, 2 Jul 2026 11:45:09 -0400 Subject: [PATCH 13/13] 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);