From 4da4443950a1a52b6938a7b4fac7d52a11234f70 Mon Sep 17 00:00:00 2001 From: Peter LaFosse Date: Sun, 19 Jul 2026 09:15:53 -0400 Subject: [PATCH 1/3] Fix legacy PE COFF symbol addresses --- view/pe/peview.cpp | 89 +++++++++++++++++++++++++++++++++++++++++++++- view/pe/peview.h | 1 + 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/view/pe/peview.cpp b/view/pe/peview.cpp index 6af6283eaf..7d3dc48c6f 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 ed79a02d16..bbad3df3e9 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); From 29efdc03b122fa867645d8058e096aa17f14ecc7 Mon Sep 17 00:00:00 2001 From: Peter LaFosse Date: Sun, 19 Jul 2026 09:31:54 -0400 Subject: [PATCH 2/3] Add legacy COFF PE regression fixture --- view/pe/tests/fixtures/README.md | 20 ++ .../fixtures/generate_legacy_coff_debug.py | 218 ++++++++++++++++++ view/pe/tests/fixtures/legacy_coff_debug.exe | Bin 0 -> 5686 bytes 3 files changed, 238 insertions(+) create mode 100644 view/pe/tests/fixtures/README.md create mode 100644 view/pe/tests/fixtures/generate_legacy_coff_debug.py create mode 100644 view/pe/tests/fixtures/legacy_coff_debug.exe diff --git a/view/pe/tests/fixtures/README.md b/view/pe/tests/fixtures/README.md new file mode 100644 index 0000000000..ead791ce85 --- /dev/null +++ b/view/pe/tests/fixtures/README.md @@ -0,0 +1,20 @@ +# 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`. 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 +``` + +The expected SHA-256 is +`457b7fdf56f480477e8fe80d1e937adaafc06ab3bc8c3f9a6ad389b8b4a067d8`. 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 0000000000..7493c8ef68 --- /dev/null +++ b/view/pe/tests/fixtures/generate_legacy_coff_debug.py @@ -0,0 +1,218 @@ +#!/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 + +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("5j>3kp4hg9MFYfdzvi72tR*JTJnda8I|4n>s$l3%h5l=*ndM^ph!1P5%8WbgoBK5Hrx+E=pt@RM9RXa+R zCPV=u=stTHBGtt7+-Z~kFTIJ)uNFN+UASNEtw_USk$rMGW;QF&);qYjjZyWMIv_so z;yvc7es=i+wIo0SBtQZrKmsH{0wh2JBtQZrKmsH{0>4TCv$th+Ii1DP{+|H;d0c-c XW|1?WIOnJAaMKZ`=8bj+ZqoV&M+XnA literal 0 HcmV?d00001 From 9d8d3bb904d8063df3e4d87e12318c72a1d51851 Mon Sep 17 00:00:00 2001 From: Peter LaFosse Date: Sun, 19 Jul 2026 09:46:21 -0400 Subject: [PATCH 3/3] Make legacy COFF fixture visually distinct --- view/pe/tests/fixtures/README.md | 11 ++++++----- .../fixtures/generate_legacy_coff_debug.py | 7 ++++++- view/pe/tests/fixtures/legacy_coff_debug.exe | Bin 5686 -> 5686 bytes 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/view/pe/tests/fixtures/README.md b/view/pe/tests/fixtures/README.md index ead791ce85..c0034cbd3b 100644 --- a/view/pe/tests/fixtures/README.md +++ b/view/pe/tests/fixtures/README.md @@ -6,9 +6,10 @@ 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`. Treating the record as an ordinary -section-relative symbol incorrectly adds the `.text` RVA of `0x1000` and -places the symbol at RVA `0x2010`. +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: @@ -16,5 +17,5 @@ Regenerate the fixture with: python3 generate_legacy_coff_debug.py legacy_coff_debug.exe ``` -The expected SHA-256 is -`457b7fdf56f480477e8fe80d1e937adaafc06ab3bc8c3f9a6ad389b8b4a067d8`. +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 index 7493c8ef68..42a3d64ba6 100644 --- a/view/pe/tests/fixtures/generate_legacy_coff_debug.py +++ b/view/pe/tests/fixtures/generate_legacy_coff_debug.py @@ -15,6 +15,7 @@ TEXT_RAW_OFFSET = 0x200 TEXT_RAW_SIZE = 0x1200 TARGET_RVA = 0x1010 +ENTRY_RVA = 0x1020 RDATA_RVA = 0x3000 RDATA_RAW_OFFSET = 0x1400 @@ -93,7 +94,7 @@ def build_image(): TEXT_RAW_SIZE, RDATA_RAW_SIZE, 0, - TARGET_RVA, + ENTRY_RVA, TEXT_RVA, RDATA_RVA, IMAGE_BASE, @@ -153,6 +154,10 @@ def build_image(): # mov eax, 42; ret target_offset = TEXT_RAW_OFFSET + TARGET_RVA - TEXT_RVA image[target_offset : target_offset + 6] = b"\xB8\x2A\x00\x00\x00\xC3" + # xor eax, eax; ret -- keep the PE entry point distinct from `legacy` so + # Binary Ninja's automatic `_start` symbol does not hide it in the UI. + entry_offset = TEXT_RAW_OFFSET + ENTRY_RVA - TEXT_RVA + image[entry_offset : entry_offset + 3] = b"\x31\xC0\xC3" # IMAGE_DEBUG_DIRECTORY. The payload is deliberately raw-only. struct.pack_into( diff --git a/view/pe/tests/fixtures/legacy_coff_debug.exe b/view/pe/tests/fixtures/legacy_coff_debug.exe index 5e0db056733998f8b2b1206cdfad073c70c1b0a8..bd5aefa256a01a364b9d97b604d37571e81f0d34 100644 GIT binary patch delta 29 lcmdm{vrT8h3Py#EE6W*~4G$dNtjP3IaFQ6yW&sg3CIGFV3L*di delta 28 kcmdm{vrT8h3PypAE6W)-TQgl0oTwl@NsMK)fQT9s0GwY5DF6Tf