diff --git a/binaryview.cpp b/binaryview.cpp index 8dfd5f6898..e98a96d7c9 100644 --- a/binaryview.cpp +++ b/binaryview.cpp @@ -1384,15 +1384,39 @@ BinaryView::BinaryView(BNBinaryView* view) bool BinaryView::InitCallback(void* ctxt) { - CallbackRef view(ctxt); - return view->Init(); + try + { + CallbackRef 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 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"); + } } @@ -1531,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; + } } diff --git a/view/macho/chained_fixups.cpp b/view/macho/chained_fixups.cpp index e71b34b800..76ca15d684 100644 --- a/view/macho/chained_fixups.cpp +++ b/view/macho/chained_fixups.cpp @@ -240,12 +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)); return { - std::string_view(&symbolData[import.name_offset]), + SymbolNameAt(symbolData, import.name_offset), 0, import.lib_ordinal > 0xF0 ? static_cast(import.lib_ordinal) : static_cast(import.lib_ordinal), (bool)import.weak_import, @@ -257,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, @@ -269,7 +278,7 @@ ImportEntry ReadChainedImportAddend64(BinaryReader& reader, std::span 0xFFF0 ? static_cast(import.lib_ordinal) : static_cast(import.lib_ordinal), (bool)import.weak_import, @@ -304,6 +313,8 @@ std::vector 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); diff --git a/view/macho/machoview.cpp b/view/macho/machoview.cpp index 7613918806..c3eebc7377 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), @@ -2268,9 +2278,22 @@ bool MachoView::InitializeHeader(MachOHeader& header, bool isMainHeader, uint64_ // Handle indirect symbols if (header.dysymtab.nindirectsyms) { - indirectSymbols.resize(header.dysymtab.nindirectsyms); - reader.Seek(header.dysymtab.indirectsymoff); - reader.Read(&indirectSymbols[0], header.dysymtab.nindirectsyms * 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&) @@ -2862,9 +2885,11 @@ 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); + if (buffer.GetLength() == 0) + return; + size_t endIndex = buffer.GetLength() - 1; struct Node { @@ -2875,7 +2900,11 @@ void MachoView::ParseExportTrie(BinaryReader& reader, linkedit_data_command expo stack.reserve(64); stack.push_back({ /* cursor */ 0, /* text */ "" }); - while (!stack.empty()) + // 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()); @@ -2884,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(); @@ -2905,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(); @@ -2922,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(); @@ -2962,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(); @@ -2990,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; @@ -3002,7 +3072,7 @@ void MachoView::ParseRebaseTable(BinaryReader& reader, MachOHeader& header, uint switch (opcode) { case RebaseOpcodeDone: - done = true; + tableDone = true; break; case RebaseOpcodeSetTypeImmediate: break; @@ -3019,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; @@ -3042,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; @@ -3112,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); @@ -3127,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; @@ -3171,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; @@ -3216,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; @@ -3335,10 +3382,14 @@ 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; - 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." || @@ -3825,8 +3876,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; diff --git a/view/macho/machoview.h b/view/macho/machoview.h index 0e0216f96b..b997af750f 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);