Skip to content

Commit 3f598a7

Browse files
Weitao-Sunclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 35a60be commit 3f598a7

2 files changed

Lines changed: 55 additions & 5 deletions

File tree

view/pe/peview.cpp

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,11 +1522,13 @@ bool PEView::Init()
15221522
size_t numImportEntries = 0;
15231523
vector<Ref<Metadata>> libraries;
15241524
vector<Ref<Metadata>> libraryFound;
1525-
uint32_t guard_1 = 0;
1526-
while (guard_1++ < 0x100)
1525+
while (true)
15271526
{
15281527
// Read in next directory entry
1529-
reader.Seek(RVAToFileOffset(dir.virtualAddress + (numImportEntries * 20)));
1528+
uint64_t entryRva = dir.virtualAddress + (uint64_t)numImportEntries * 20;
1529+
if (!IsRVARangeBackedByFile(entryRva, 20))
1530+
break;
1531+
reader.Seek(RVAToFileOffset(entryRva));
15301532
PEImportDirectoryEntry importDirEntry;
15311533
importDirEntry.lookup = reader.Read32();
15321534
importDirEntry.timestamp = reader.Read32();
@@ -1615,9 +1617,26 @@ bool PEView::Init()
16151617
// We should make this second unused data a structure containing this information information
16161618
// and default it to collapsed...IDA Just doesn't show anything at all
16171619
m_logger->LogDebug("Name: %s\n", dllName.c_str());
1618-
uint32_t guard_2 = 0;
1619-
while (guard_2++ < 0x1000)
1620+
const uint32_t importEntrySize = m_is64 ? 8 : 4;
1621+
1622+
// Find the section containing entryOffset once so the inner loop needs no per-entry section scan.
1623+
uint64_t importSecEnd = 0;
1624+
for (const auto& sec : m_sections)
16201625
{
1626+
if ((uint64_t)entryOffset >= sec.virtualAddress &&
1627+
(uint64_t)entryOffset < (uint64_t)sec.virtualAddress + sec.sizeOfRawData &&
1628+
sec.virtualSize != 0)
1629+
{
1630+
// Only trust this section's declared extent up to what the file actually backs.
1631+
if ((uint64_t)sec.pointerToRawData + sec.sizeOfRawData <= (uint64_t)GetParentView()->GetLength())
1632+
importSecEnd = (uint64_t)sec.virtualAddress + sec.sizeOfRawData;
1633+
break;
1634+
}
1635+
}
1636+
while (true)
1637+
{
1638+
if ((uint64_t)entryOffset + importEntrySize > importSecEnd)
1639+
break;
16211640
uint64_t entry;
16221641
bool isOrdinal;
16231642
if (m_is64)
@@ -3453,6 +3472,36 @@ uint32_t PEView::GetRVACharacteristics(uint64_t offset)
34533472
}
34543473

34553474

3475+
// Returns true only if the entire range [rva, rva+size) maps to file-backed data.
3476+
bool PEView::IsRVARangeBackedByFile(uint64_t rva, uint64_t size) const
3477+
{
3478+
if (size == 0)
3479+
return false;
3480+
if (size > UINT64_MAX - rva)
3481+
return false;
3482+
for (uint64_t offset = rva; offset < rva + size; offset++)
3483+
{
3484+
bool found = false;
3485+
for (const auto& i : m_sections)
3486+
{
3487+
if (offset >= (uint64_t)i.virtualAddress &&
3488+
offset < (uint64_t)i.virtualAddress + (uint64_t)i.sizeOfRawData &&
3489+
i.virtualSize != 0)
3490+
{
3491+
uint64_t fileOfs = (uint64_t)i.pointerToRawData + (offset - (uint64_t)i.virtualAddress);
3492+
if (!GetParentView()->IsOffsetBackedByFile(fileOfs))
3493+
return false;
3494+
found = true;
3495+
break;
3496+
}
3497+
}
3498+
if (!found)
3499+
return false;
3500+
}
3501+
return true;
3502+
}
3503+
3504+
34563505
string PEView::ReadString(uint64_t rva)
34573506
{
34583507
uint64_t offset = RVAToFileOffset(rva);

view/pe/peview.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ namespace BinaryNinja
467467

468468
uint64_t RVAToFileOffset(uint64_t rva, bool except = true);
469469
uint32_t GetRVACharacteristics(uint64_t rva);
470+
bool IsRVARangeBackedByFile(uint64_t rva, uint64_t size) const;
470471
std::string ReadString(uint64_t rva);
471472
uint16_t Read16(uint64_t rva);
472473
uint32_t Read32(uint64_t rva);

0 commit comments

Comments
 (0)