Skip to content

Commit 6acb6ec

Browse files
Weitao-Sunrbranclaude
authored
Extend rbran's parser robustness fixes with file-backed bounds and configurable limits (#8303)
Improve Mach-O parsing robustness and memory safety by tightening bounds checks, correctly accounting for fat-binary slice offsets, and avoiding empty-vector access. Rework the rebase/bind entry limit to derive from slice and pointer size, and centralize budget enforcement so all relocation opcodes are consistently bounded. Also simplify several reads, naming, and redundant exception handling. Co-authored-by: rbran <git@rubens.io> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6f8ff5a commit 6acb6ec

4 files changed

Lines changed: 216 additions & 111 deletions

File tree

binaryview.cpp

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,15 +1384,39 @@ BinaryView::BinaryView(BNBinaryView* view)
13841384

13851385
bool BinaryView::InitCallback(void* ctxt)
13861386
{
1387-
CallbackRef<BinaryView> view(ctxt);
1388-
return view->Init();
1387+
try
1388+
{
1389+
CallbackRef<BinaryView> view(ctxt);
1390+
return view->Init();
1391+
}
1392+
catch (const std::exception& e)
1393+
{
1394+
LogError("BinaryView::Init failed: %s", e.what());
1395+
return false;
1396+
}
1397+
catch (...)
1398+
{
1399+
LogError("BinaryView::Init failed with unknown exception");
1400+
return false;
1401+
}
13891402
}
13901403

13911404

13921405
void BinaryView::OnAfterSnapshotDataAppliedCallback(void* ctxt)
13931406
{
1394-
CallbackRef<BinaryView> view(ctxt);
1395-
view->OnAfterSnapshotDataApplied();
1407+
try
1408+
{
1409+
CallbackRef<BinaryView> view(ctxt);
1410+
view->OnAfterSnapshotDataApplied();
1411+
}
1412+
catch (const std::exception& e)
1413+
{
1414+
LogError("BinaryView::OnAfterSnapshotDataApplied failed: %s", e.what());
1415+
}
1416+
catch (...)
1417+
{
1418+
LogError("BinaryView::OnAfterSnapshotDataApplied failed with unknown exception");
1419+
}
13961420
}
13971421

13981422

@@ -1531,9 +1555,22 @@ size_t BinaryView::GetAddressSizeCallback(void* ctxt)
15311555

15321556
bool BinaryView::SaveCallback(void* ctxt, BNFileAccessor* file)
15331557
{
1534-
CallbackRef<BinaryView> view(ctxt);
1535-
CoreFileAccessor accessor(file);
1536-
return view->PerformSave(&accessor);
1558+
try
1559+
{
1560+
CallbackRef<BinaryView> view(ctxt);
1561+
CoreFileAccessor accessor(file);
1562+
return view->PerformSave(&accessor);
1563+
}
1564+
catch (const std::exception& e)
1565+
{
1566+
LogError("BinaryView::Save failed: %s", e.what());
1567+
return false;
1568+
}
1569+
catch (...)
1570+
{
1571+
LogError("BinaryView::Save failed with unknown exception");
1572+
return false;
1573+
}
15371574
}
15381575

15391576

view/macho/chained_fixups.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,12 +240,21 @@ auto FixupReaderForFormat(int format) -> std::pair<uint64_t, FixupInfo>(*)(Binar
240240
throw std::invalid_argument("Unknown chained pointer format: " + std::to_string(format));
241241
}
242242

243+
// Returns the NUL-terminated string starting at `offset` within `symbolData`, or an
244+
// empty view if `offset` does not fall within `symbolData`.
245+
std::string_view SymbolNameAt(std::span<const char> symbolData, uint32_t offset)
246+
{
247+
if (symbolData.size() <= offset)
248+
return std::string_view();
249+
return std::string_view(&symbolData[offset], strnlen(&symbolData[offset], symbolData.size() - offset));
250+
}
251+
243252
ImportEntry ReadChainedImport32(BinaryReader& reader, std::span<const char> symbolData)
244253
{
245254
dyld_chained_import import;
246255
reader.Read(&import, sizeof(import));
247256
return {
248-
std::string_view(&symbolData[import.name_offset]),
257+
SymbolNameAt(symbolData, import.name_offset),
249258
0,
250259
import.lib_ordinal > 0xF0 ? static_cast<int8_t>(import.lib_ordinal) : static_cast<int32_t>(import.lib_ordinal),
251260
(bool)import.weak_import,
@@ -257,7 +266,7 @@ ImportEntry ReadChainedImportAddend32(BinaryReader& reader, std::span<const char
257266
dyld_chained_import_addend import;
258267
reader.Read(&import, sizeof(import));
259268
return {
260-
std::string_view(&symbolData[import.name_offset]),
269+
SymbolNameAt(symbolData, import.name_offset),
261270
static_cast<uint32_t>(import.addend),
262271
import.lib_ordinal > 0xF0 ? static_cast<int8_t>(import.lib_ordinal) : static_cast<int32_t>(import.lib_ordinal),
263272
(bool)import.weak_import,
@@ -269,7 +278,7 @@ ImportEntry ReadChainedImportAddend64(BinaryReader& reader, std::span<const char
269278
dyld_chained_import_addend64 import;
270279
reader.Read(&import, sizeof(import));
271280
return {
272-
std::string_view(&symbolData[import.name_offset]),
281+
SymbolNameAt(symbolData, import.name_offset),
273282
import.addend,
274283
import.lib_ordinal > 0xFFF0 ? static_cast<int16_t>(import.lib_ordinal) : static_cast<int32_t>(import.lib_ordinal),
275284
(bool)import.weak_import,
@@ -304,6 +313,8 @@ std::vector<ImportEntry> ChainedFixupProcessor::ProcessImports() const
304313

305314
auto header = ReadHeader(reader);
306315

316+
if (header.symbols_offset >= m_fixupsSize)
317+
return imports;
307318
uint64_t symbolDataSize = m_fixupsSize - header.symbols_offset;
308319
m_symbolData.resize(symbolDataSize);
309320
m_raw->Read(&m_symbolData[0], OffsetInFixups(header.symbols_offset), symbolDataSize);
@@ -435,7 +446,7 @@ void ChainedFixupProcessor::ProcessChainsInSegment(const dyld_chained_starts_in_
435446

436447
bool done = false;
437448
while (!done)
438-
{
449+
{
439450
uint64_t position = reader.GetOffset();
440451
auto [raw, fixupInfo] = fixupReader(reader);
441452

0 commit comments

Comments
 (0)