diff --git a/view/pe/peview.cpp b/view/pe/peview.cpp index 6af6283ea..7d3dc48c6 100644 --- a/view/pe/peview.cpp +++ b/view/pe/peview.cpp @@ -1358,6 +1358,7 @@ bool PEView::Init() // Process COFF symbol table if (header.coffSymbolCount) { + const bool coffSymbolValuesAreRvas = CoffSymbolValuesAreRvas(header); BinaryReader stringReader(GetParentView(), LittleEndian); uint64_t stringTableBase = header.coffSymbolTable + (header.coffSymbolCount * 18); stringReader.Seek(stringTableBase); @@ -1386,7 +1387,8 @@ bool PEView::Init() break; default: if (size_t(e_scnum - 1) < m_sections.size()) - virtualAddress = m_sections[size_t(e_scnum - 1)].virtualAddress + e_value; + virtualAddress = coffSymbolValuesAreRvas ? e_value : + m_sections[size_t(e_scnum - 1)].virtualAddress + e_value; break; } @@ -3440,6 +3442,91 @@ uint64_t PEView::RVAToFileOffset(uint64_t offset, bool except) } +bool PEView::CoffSymbolValuesAreRvas(const PEHeader& header) +{ + /* + * A normal PE COFF symbol table stores section-relative symbol values. Legacy + * link.exe /DEBUGTYPE:COFF output instead wraps the table in an + * IMAGE_COFF_SYMBOLS_HEADER and stores image-relative values. Keep the normal + * interpretation used for https://github.com/Vector35/binaryninja-api/issues/1956, + * and select the legacy interpretation only when the wrapper exactly identifies + * the PE header's table. See: + * + * https://github.com/Vector35/binaryninja-api/issues/4308 + * https://learn.microsoft.com/en-us/archive/msdn-magazine/2002/march/inside-windows-an-in-depth-look-into-the-win32-portable-executable-file-format-part-2 + * https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-image_coff_symbols_header + * https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#debug-directory-image-only + * + * AddressOfRawData is allowed to be zero for debug data outside an image + * section, so this check deliberately uses PointerToRawData. + */ + if (!header.coffSymbolTable || !header.coffSymbolCount || + (m_dataDirs.size() <= IMAGE_DIRECTORY_ENTRY_DEBUG)) + return false; + + const PEDataDirectory& dir = m_dataDirs[IMAGE_DIRECTORY_ENTRY_DEBUG]; + if (!dir.virtualAddress || (dir.size < sizeof(DebugDirectory))) + return false; + + const uint64_t fileEnd = GetParentView()->GetEnd(); + uint64_t debugDirectoryOffset; + try + { + debugDirectoryOffset = RVAToFileOffset(dir.virtualAddress); + } + catch (const std::exception&) + { + return false; + } + + const uint64_t debugDirectorySize = dir.size - (dir.size % sizeof(DebugDirectory)); + if ((debugDirectoryOffset > fileEnd) || (debugDirectorySize > fileEnd - debugDirectoryOffset)) + return false; + + BinaryReader reader(GetParentView(), LittleEndian); + for (uint64_t offset = 0; offset < debugDirectorySize; offset += sizeof(DebugDirectory)) + { + reader.Seek(debugDirectoryOffset + offset); + reader.Read32(); // Characteristics + reader.Read32(); // TimeDateStamp + reader.Read16(); // MajorVersion + reader.Read16(); // MinorVersion + const uint32_t type = reader.Read32(); + const uint32_t sizeOfData = reader.Read32(); + reader.Read32(); // AddressOfRawData + const uint32_t pointerToRawData = reader.Read32(); + + constexpr uint64_t coffSymbolsHeaderSize = 8 * sizeof(uint32_t); + if ((type != IMAGE_DEBUG_TYPE_COFF) || !pointerToRawData || + (sizeOfData < coffSymbolsHeaderSize) || (pointerToRawData > fileEnd) || + (sizeOfData > fileEnd - pointerToRawData)) + continue; + + reader.Seek(pointerToRawData); + const uint32_t numberOfSymbols = reader.Read32(); + const uint32_t lvaToFirstSymbol = reader.Read32(); + reader.Read32(); // NumberOfLinenumbers + reader.Read32(); // LvaToFirstLinenumber + reader.Read32(); // RvaToFirstByteOfCode + reader.Read32(); // RvaToLastByteOfCode + reader.Read32(); // RvaToFirstByteOfData + reader.Read32(); // RvaToLastByteOfData + + const uint64_t symbolBytes = uint64_t(numberOfSymbols) * 18; + if ((numberOfSymbols != header.coffSymbolCount) || + (uint64_t(pointerToRawData) + lvaToFirstSymbol != header.coffSymbolTable) || + (lvaToFirstSymbol < coffSymbolsHeaderSize) || (lvaToFirstSymbol > sizeOfData) || + (symbolBytes > sizeOfData - lvaToFirstSymbol)) + continue; + + m_logger->LogDebug("Using image-relative values from legacy COFF debug symbol table"); + return true; + } + + return false; +} + + uint32_t PEView::GetRVACharacteristics(uint64_t offset) { for (auto& i : m_sections) diff --git a/view/pe/peview.h b/view/pe/peview.h index ed79a02d1..bbad3df3e 100644 --- a/view/pe/peview.h +++ b/view/pe/peview.h @@ -466,6 +466,7 @@ namespace BinaryNinja Ref m_symExternMappingMetadata; uint64_t RVAToFileOffset(uint64_t rva, bool except = true); + bool CoffSymbolValuesAreRvas(const PEHeader& header); uint32_t GetRVACharacteristics(uint64_t rva); std::string ReadString(uint64_t rva); uint16_t Read16(uint64_t rva); diff --git a/view/pe/tests/fixtures/README.md b/view/pe/tests/fixtures/README.md new file mode 100644 index 000000000..c0034cbd3 --- /dev/null +++ b/view/pe/tests/fixtures/README.md @@ -0,0 +1,21 @@ +# Legacy COFF debug fixture + +`legacy_coff_debug.exe` is a minimal synthetic PE32 image for +[binaryninja-api#4308](https://github.com/Vector35/binaryninja-api/issues/4308). +It contains a raw-only `IMAGE_DEBUG_TYPE_COFF` entry and an +`IMAGE_COFF_SYMBOLS_HEADER` whose symbol table matches the PE file header. + +The `legacy` function is physically located at RVA `0x1010`, and its COFF +symbol value is also `0x1010`. The separate PE entry point is at RVA `0x1020`, +so Binary Ninja's automatic `_start` symbol does not obscure `legacy` in the +UI. Treating the record as an ordinary section-relative symbol incorrectly +adds the `.text` RVA of `0x1000` and places the symbol at RVA `0x2010`. + +Regenerate the fixture with: + +```sh +python3 generate_legacy_coff_debug.py legacy_coff_debug.exe +``` + +Expected SHA-256: +`7d3d2b3a45405e542d4c5644712cd11b27d5c08b968f7020e2960ecffb8dcba7`. diff --git a/view/pe/tests/fixtures/generate_legacy_coff_debug.py b/view/pe/tests/fixtures/generate_legacy_coff_debug.py new file mode 100644 index 000000000..42a3d64ba --- /dev/null +++ b/view/pe/tests/fixtures/generate_legacy_coff_debug.py @@ -0,0 +1,223 @@ +#!/usr/bin/env python3 +"""Generate a minimal PE32 image with a legacy /DEBUGTYPE:COFF table.""" + +import argparse +import struct +from pathlib import Path + + +FILE_ALIGNMENT = 0x200 +SECTION_ALIGNMENT = 0x1000 +IMAGE_BASE = 0x400000 + +PE_OFFSET = 0x80 +TEXT_RVA = 0x1000 +TEXT_RAW_OFFSET = 0x200 +TEXT_RAW_SIZE = 0x1200 +TARGET_RVA = 0x1010 +ENTRY_RVA = 0x1020 + +RDATA_RVA = 0x3000 +RDATA_RAW_OFFSET = 0x1400 +RDATA_RAW_SIZE = 0x200 + +DEBUG_DIRECTORY_RVA = RDATA_RVA +DEBUG_DIRECTORY_SIZE = 28 +COFF_DEBUG_OFFSET = 0x1600 +COFF_HEADER_SIZE = 32 +COFF_SYMBOL_OFFSET = COFF_DEBUG_OFFSET + COFF_HEADER_SIZE +COFF_SYMBOL_COUNT = 1 +COFF_SYMBOL_SIZE = 18 +COFF_STRING_TABLE_SIZE = 4 +COFF_DEBUG_SIZE = COFF_HEADER_SIZE + COFF_SYMBOL_SIZE + COFF_STRING_TABLE_SIZE +FILE_SIZE = COFF_DEBUG_OFFSET + COFF_DEBUG_SIZE + + +def write_section_header( + image, + offset, + name, + virtual_size, + virtual_address, + raw_size, + raw_offset, + characteristics, +): + struct.pack_into( + "<8sIIIIIIHHI", + image, + offset, + name.encode("ascii").ljust(8, b"\0"), + virtual_size, + virtual_address, + raw_size, + raw_offset, + 0, + 0, + 0, + 0, + characteristics, + ) + + +def build_image(): + image = bytearray(FILE_SIZE) + + image[0:2] = b"MZ" + struct.pack_into("