From 99059c4ba280841a073f0abf5b7c9b01bfea3ba0 Mon Sep 17 00:00:00 2001 From: rbran Date: Sun, 14 Jun 2026 09:27:58 -0300 Subject: [PATCH 1/3] 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 35a60be3c41affecf10a9516c68fe50997840691 Mon Sep 17 00:00:00 2001 From: rbran Date: Mon, 15 Jun 2026 16:34:33 -0300 Subject: [PATCH 2/3] 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 3f598a7c352a0e68a0c0aeec627f59b77797b3a0 Mon Sep 17 00:00:00 2001 From: Weitao Sun Date: Wed, 1 Jul 2026 13:34:45 -0400 Subject: [PATCH 3/3] 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);