Extend rbran's parser robustness fixes with file-backed bounds and configurable limits (PE)

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);
@@ -1525,7 +1525,10 @@ bool PEView::Init()
 			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();
@@ -1614,8 +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());
+				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)
@@ -3451,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);

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);