From b9ee8fe05774570380659fba81cddb6be0a29576 Mon Sep 17 00:00:00 2001
From: morluto <76467478+morluto@users.noreply.github.com>
Date: Mon, 26 Oct 2026 15:20:02 +0800
Subject: [PATCH] refactor(pe): centralize fixed COFF header decoding

---
 src/application/BinaryTargetResolver.ts       |  7 ++--
 src/artifacts/DesktopFileInspection.ts        | 12 +++++--
 src/domain/binaryTarget.ts                    | 18 +++++++---
 src/domain/native/nativeAotPe.ts              |  9 +++--
 src/domain/peCoffHeader.test.ts               | 33 +++++++++++++++++++
 src/domain/peCoffHeader.ts                    | 16 +++++++++
 src/dotnet/ManagedPeReader.ts                 | 19 ++++++++---
 src/electron/ElectronFuseReader.ts            |  6 +++-
 src/native/metadata/NativeSectionDirectory.ts | 15 ++++++---
 src/native/metadata/PeImage.ts                | 12 +++++--
 src/native/pe/PeResourceLayout.ts             | 12 +++++--
 src/windows/WindowsNativeLoader.ts            |  5 ++-
 12 files changed, 135 insertions(+), 29 deletions(-)
 create mode 100644 src/domain/peCoffHeader.test.ts
 create mode 100644 src/domain/peCoffHeader.ts

diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts
index bd190428d6..1de48152fd 100644
--- a/src/application/BinaryTargetResolver.ts
+++ b/src/application/BinaryTargetResolver.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../domain/peCoffHeader.js";
 import {
   regularFileCancellation,
   RegularFileCleanupFailure,
@@ -1285,9 +1286,11 @@ const readPeMetadata = async (
     return err("invalid Windows new-header signature in MZ image");
   if (["NE", "LE", "LX"].includes(windowsHeader.signature))
     return err(`unsupported ${windowsHeader.signature} executable in MZ image`);
-  const optionalHeaderSize = fileHeader.readUInt16LE(20);
+  const coff = readPeCoffHeader(fileHeader.subarray(4));
+  if (!coff.ok) return err("invalid or truncated PE header");
+  const optionalHeaderSize = coff.value.optionalHeaderSize;
   if (optionalHeaderSize > 4096) return err("invalid PE optional header size");
-  const numberOfSections = fileHeader.readUInt16LE(6);
+  const numberOfSections = coff.value.sectionCount;
   const record = Buffer.alloc(24 + optionalHeaderSize);
   const recordRead = await handle.read(record, 0, record.length, offset);
   throwIfTargetResolutionCancelled(signal);
diff --git a/src/artifacts/DesktopFileInspection.ts b/src/artifacts/DesktopFileInspection.ts
index 1669cd53fe..cfc7f81f9d 100644
--- a/src/artifacts/DesktopFileInspection.ts
+++ b/src/artifacts/DesktopFileInspection.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../domain/peCoffHeader.js";
 import {
   hasGodotPckHeader,
   readGodotPckHeaderFacts,
@@ -208,7 +209,12 @@ const inspectPeFile = async (
   const peOffset = prefix.readUInt32LE(0x3c);
   const coff = await read(peOffset, 24);
   if (coff.toString("latin1", 0, 4) !== "PE\0\0") return;
-  const optionalSize = coff.readUInt16LE(20);
+  const header = readPeCoffHeader(coff.subarray(4));
+  if (!header.ok) {
+    row.issues.push(header.error.message);
+    return;
+  }
+  const optionalSize = header.value.optionalHeaderSize;
   if (optionalSize > 4096)
     throw new ArtifactReaderFailure(
       "format",
@@ -224,7 +230,7 @@ const inspectPeFile = async (
   fact({
     kind: "native-image-header",
     offset: peOffset,
-    value: { format: "pe", machine: coff.readUInt16LE(4) },
+    value: { format: "pe", machine: header.value.machine },
   });
   if (!parsed.value.managed) return;
   const directories = optional.readUInt16LE(0) === 0x20b ? 112 : 96;
@@ -236,7 +242,7 @@ const inspectPeFile = async (
     value: { rva: cliRva, bytes: cliSize },
   });
   if (unityManagedDirectory === null) return;
-  const sectionCount = coff.readUInt16LE(6);
+  const sectionCount = header.value.sectionCount;
   const sections = await read(peOffset + 24 + optionalSize, sectionCount * 40);
   const mapped = (rva: number, bytes: number) =>
     peRvaToFileOffset(
diff --git a/src/domain/binaryTarget.ts b/src/domain/binaryTarget.ts
index 89755f26aa..8a9a5f3d09 100644
--- a/src/domain/binaryTarget.ts
+++ b/src/domain/binaryTarget.ts
@@ -9,6 +9,7 @@ import type {
   UniversalMachOSlice,
 } from "./binaryTargetTypes.js";
 import { err, ok, type Result } from "./result.js";
+import { readPeCoffHeader } from "./peCoffHeader.js";
 import { readPeSectionHeader } from "./peSectionHeader.js";
 import { readElfFixedHeader } from "./elfFixedHeader.js";
 import {
@@ -489,13 +490,18 @@ export const parsePeRecord = (
     record.toString("binary", 0, 4) !== "PE\u0000\u0000"
   )
     return err("invalid or truncated PE header");
-  const machineType = record.readUInt16LE(4);
+  const coff = readPeCoffHeader(record.subarray(4, 24));
+  if (!coff.ok) return err("invalid or truncated PE header");
+  const {
+    machine: machineType,
+    optionalHeaderSize,
+    characteristics,
+  } = coff.value;
   const machine = decodePeMachine(machineType);
   if (machine === undefined)
     return err(
       `unsupported PE architecture (COFF machine 0x${machineType.toString(16).padStart(4, "0")})`,
     );
-  const optionalHeaderSize = record.readUInt16LE(20);
   if (optionalHeaderSize > 4096 || record.length < 24 + optionalHeaderSize)
     return err("invalid or truncated PE optional header");
   const optionalHeader = record.subarray(24, 24 + optionalHeaderSize);
@@ -503,7 +509,6 @@ export const parsePeRecord = (
   if (!facts.ok) return err(facts.error.message);
   if (facts.value.machine === null) return err("unsupported PE architecture");
   const { architecture } = facts.value.machine;
-  const characteristics = record.readUInt16LE(22);
   return ok({
     format: "pe",
     peMachine: machineType,
@@ -623,8 +628,11 @@ export const parsePeCodeViewFromBuffer = (
   peOffset: number,
 ): PeCodeViewRecord | undefined => {
   if (fileBytes.length < peOffset + 24) return undefined;
-  const numberOfSections = fileBytes.readUInt16LE(peOffset + 6);
-  const optionalHeaderSize = fileBytes.readUInt16LE(peOffset + 20);
+  const coff = readPeCoffHeader(
+    fileBytes.subarray(peOffset + 4, peOffset + 24),
+  );
+  if (!coff.ok) return undefined;
+  const { sectionCount: numberOfSections, optionalHeaderSize } = coff.value;
   if (
     optionalHeaderSize > 4096 ||
     fileBytes.length < peOffset + 24 + optionalHeaderSize
diff --git a/src/domain/native/nativeAotPe.ts b/src/domain/native/nativeAotPe.ts
index 2e816408aa..325969b857 100644
--- a/src/domain/native/nativeAotPe.ts
+++ b/src/domain/native/nativeAotPe.ts
@@ -1,4 +1,5 @@
 import { createHash } from "node:crypto";
+import { readPeCoffHeader } from "../peCoffHeader.js";
 import { readPeSectionHeader } from "../peSectionHeader.js";
 import type { NativeMetadataRecovery } from "./nativeMetadataRecovery.js";
 
@@ -493,13 +494,15 @@ class PeImage {
     this.#range(peOffset, 24);
     if (bytes.toString("latin1", peOffset, peOffset + 4) !== "PE\0\0")
       throw new Error("Invalid PE signature");
-    this.#machine = this.#u16(peOffset + 4);
-    const count = this.#u16(peOffset + 6);
+    const coff = readPeCoffHeader(bytes.subarray(peOffset + 4, peOffset + 24));
+    if (!coff.ok) throw new Error(coff.error.message);
+    this.#machine = coff.value.machine;
+    const { sectionCount: count, optionalHeaderSize: optionalSize } =
+      coff.value;
     // Microsoft documents a 96-section Windows image-loader limit. This
     // parser accepts executable PE images, not arbitrary COFF object files.
     if (count > 96)
       throw new Error("PE image exceeds the Windows 96-section limit");
-    const optionalSize = this.#u16(peOffset + 20);
     const optional = peOffset + 24;
     this.#range(optional, optionalSize);
     // Mapping fields through SizeOfHeaders must belong to this header,
diff --git a/src/domain/peCoffHeader.test.ts b/src/domain/peCoffHeader.test.ts
new file mode 100644
index 0000000000..46ef8c2e10
--- /dev/null
+++ b/src/domain/peCoffHeader.test.ts
@@ -0,0 +1,33 @@
+import { expect, it } from "vitest";
+import { readPeCoffHeader } from "./peCoffHeader.js";
+
+it("retains raw COFF declarations from a borrowed record without admission policy", () => {
+  const source = Buffer.alloc(39, 0xa5);
+  const record = source.subarray(7, 27);
+  record.writeUInt16LE(0xffff, 0);
+  record.writeUInt16LE(0x1234, 2);
+  record.writeUInt16LE(0xfedc, 16);
+  record.writeUInt16LE(0x89ab, 18);
+  const before = Buffer.from(source);
+  expect(readPeCoffHeader(source.subarray(7))).toEqual({
+    ok: true,
+    value: {
+      machine: 0xffff,
+      sectionCount: 0x1234,
+      optionalHeaderSize: 0xfedc,
+      characteristics: 0x89ab,
+    },
+  });
+  expect(source.equals(before)).toBe(true);
+});
+
+it.each(Array.from({ length: 20 }, (_, length) => length))(
+  "reports a truncated %i-byte COFF record without reading outside it",
+  (length) => {
+    const source = Buffer.alloc(40, 0xff);
+    expect(readPeCoffHeader(source.subarray(7, 7 + length))).toEqual({
+      ok: false,
+      error: { kind: "malformed", message: "PE COFF header is truncated" },
+    });
+  },
+);
diff --git a/src/domain/peCoffHeader.ts b/src/domain/peCoffHeader.ts
new file mode 100644
index 0000000000..2101e90cde
--- /dev/null
+++ b/src/domain/peCoffHeader.ts
@@ -0,0 +1,16 @@
+import { err, ok } from "./result.js";
+
+/** Fixed 20-byte COFF record, excluding the PE signature; callers own admission. */
+export const readPeCoffHeader = (bytes: Buffer) => {
+  if (bytes.length < 20)
+    return err({
+      kind: "malformed" as const,
+      message: "PE COFF header is truncated",
+    });
+  return ok({
+    machine: bytes.readUInt16LE(0),
+    sectionCount: bytes.readUInt16LE(2),
+    optionalHeaderSize: bytes.readUInt16LE(16),
+    characteristics: bytes.readUInt16LE(18),
+  });
+};
diff --git a/src/dotnet/ManagedPeReader.ts b/src/dotnet/ManagedPeReader.ts
index db5ce01d9a..fe5acc1c5d 100644
--- a/src/dotnet/ManagedPeReader.ts
+++ b/src/dotnet/ManagedPeReader.ts
@@ -1,4 +1,5 @@
 import type { ManagedParseIssue } from "../domain/managed/managedArtifact.js";
+import { readPeCoffHeader } from "../domain/peCoffHeader.js";
 import { decodePeMachine } from "../domain/peMachine.js";
 import {
   managedFailure,
@@ -299,8 +300,20 @@ const readCoffHeader = (bytes: Buffer): CoffHeader => {
       peOffset,
     );
   const coff = peOffset + 4;
-  const machine = bytes.readUInt16LE(coff);
-  const sectionCount = bytes.readUInt16LE(coff + 2);
+  const parsed = readPeCoffHeader(bytes.subarray(coff, coff + 20));
+  if (!parsed.ok)
+    throw managedFailure(
+      "invalid-directory",
+      "pe.coff-header",
+      parsed.error.message,
+      peOffset,
+    );
+  const {
+    machine,
+    sectionCount,
+    optionalHeaderSize: optionalSize,
+    characteristics,
+  } = parsed.value;
   if (sectionCount === 0)
     throw managedFailure(
       "invalid-directory",
@@ -308,8 +321,6 @@ const readCoffHeader = (bytes: Buffer): CoffHeader => {
       "PE section count must be positive",
       coff + 2,
     );
-  const optionalSize = bytes.readUInt16LE(coff + 16);
-  const characteristics = bytes.readUInt16LE(coff + 18);
   return { coff, machine, sectionCount, optionalSize, characteristics };
 };
 
diff --git a/src/electron/ElectronFuseReader.ts b/src/electron/ElectronFuseReader.ts
index b0a50cd698..aef40d00d8 100644
--- a/src/electron/ElectronFuseReader.ts
+++ b/src/electron/ElectronFuseReader.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../domain/peCoffHeader.js";
 import { readElfFixedHeader } from "../domain/elfFixedHeader.js";
 import { decodePeMachine } from "../domain/peMachine.js";
 import type { ResolvedElectronFuseFile } from "./ElectronFusePath.js";
@@ -500,8 +501,11 @@ const identifyPe = async (
     return undefined;
   // A signature establishes the existing scan boundary; only a complete COFF
   // header establishes its machine. Managed OS overrides need separate proof.
+  const coff = readPeCoffHeader(bytes.subarray(4));
   const machine =
-    bytes.length === 24 ? decodePeMachine(bytes.readUInt16LE(4)) : undefined;
+    bytes.length === 24 && coff.ok
+      ? decodePeMachine(coff.value.machine)
+      : undefined;
   return {
     architecture:
       machine?.nativeOsOverride === false ? machine.architecture : null,
diff --git a/src/native/metadata/NativeSectionDirectory.ts b/src/native/metadata/NativeSectionDirectory.ts
index cd62798c18..4a6dd31484 100644
--- a/src/native/metadata/NativeSectionDirectory.ts
+++ b/src/native/metadata/NativeSectionDirectory.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../../domain/peCoffHeader.js";
 import { readPeSectionHeader } from "../../domain/peSectionHeader.js";
 import { readElfTableDeclarations } from "../../domain/elfDeclaredRanges.js";
 import { readPeOptionalHeaderStructure } from "../../domain/peOptionalHeader.js";
@@ -274,7 +275,14 @@ export function* readPeSectionDirectory(
   if (pe < 64) reject("PE signature overlaps the DOS header");
   const coff = new BinaryReader(yield request(fileSize, BigInt(pe), 24n), true);
   if (coff.u32(0) !== 0x4550) reject("PE signature is invalid");
-  const optionalSize = coff.u16(20);
+  const parsedCoff = readPeCoffHeader(coff.bytes.subarray(4, 24));
+  if (!parsedCoff.ok) return reject(parsedCoff.error.message);
+  const {
+    sectionCount: count,
+    optionalHeaderSize: optionalSize,
+    machine,
+    characteristics,
+  } = parsedCoff.value;
   const optional = readPeOptionalHeaderStructure(
     yield request(fileSize, BigInt(pe + 24), BigInt(optionalSize)),
   );
@@ -293,7 +301,6 @@ export function* readPeSectionDirectory(
     }
   }
   const { bits } = optional.value;
-  const count = coff.u16(6);
   const start = pe + 24 + optionalSize;
   const table = yield request(fileSize, BigInt(start), BigInt(count) * 40n);
   const sections: NativeSection[] = [];
@@ -328,8 +335,8 @@ export function* readPeSectionDirectory(
     elf_os_abi: null,
     bits,
     little: true,
-    machine: coff.u16(4),
-    flags: coff.u16(22),
+    machine,
+    flags: characteristics,
     programs: [],
     sections,
   };
diff --git a/src/native/metadata/PeImage.ts b/src/native/metadata/PeImage.ts
index ce8927da03..bf0011c7cc 100644
--- a/src/native/metadata/PeImage.ts
+++ b/src/native/metadata/PeImage.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../../domain/peCoffHeader.js";
 import { readPeOptionalHeader } from "../../domain/peMachine.js";
 import { readPeSectionHeader } from "../../domain/peSectionHeader.js";
 import {
@@ -74,8 +75,14 @@ export const readPeImage = (bytes: Buffer): PeImage => {
   const reader = new BinaryReader(bytes, true);
   const pe = peHeaderOffset(reader);
   const optional = pe + 24;
-  const optionalSize = reader.u16(pe + 20);
-  const machine = reader.u16(pe + 4);
+  const coff = readPeCoffHeader(bytes.subarray(pe + 4, pe + 24));
+  if (!coff.ok)
+    throw new BinaryFormatFailure(coff.error.kind, coff.error.message);
+  const {
+    machine,
+    sectionCount: count,
+    optionalHeaderSize: optionalSize,
+  } = coff.value;
   const facts = readPeOptionalHeader(
     reader.range(optional, optionalSize, "PE optional header"),
     machine,
@@ -95,7 +102,6 @@ export const readPeImage = (bytes: Buffer): PeImage => {
     offset: 0,
     size: Math.min(sizeOfHeaders, bytes.length),
   };
-  const count = reader.u16(pe + 6);
   const start = reader.table(
     BigInt(optional + optionalSize),
     count,
diff --git a/src/native/pe/PeResourceLayout.ts b/src/native/pe/PeResourceLayout.ts
index f8a615c4c5..a04b679ad3 100644
--- a/src/native/pe/PeResourceLayout.ts
+++ b/src/native/pe/PeResourceLayout.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../../domain/peCoffHeader.js";
 import { readPeSectionHeader } from "../../domain/peSectionHeader.js";
 import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js";
 import {
@@ -84,8 +85,13 @@ export const readPeHeaders = (bytes: Buffer, fileSize = bytes.length) => {
   const pe = bytes.readUInt32LE(60);
   requirePeRange(bytes, pe, 24);
   if (bytes.readUInt32LE(pe) !== 0x4550) peFailure("Expected a PE signature.");
-  const sectionCount = bytes.readUInt16LE(pe + 6);
-  const optionalSize = bytes.readUInt16LE(pe + 20);
+  const coff = readPeCoffHeader(bytes.subarray(pe + 4, pe + 24));
+  if (!coff.ok) return peFailure(coff.error.message);
+  const {
+    sectionCount,
+    optionalHeaderSize: optionalSize,
+    machine,
+  } = coff.value;
   const optionalHeaderOffset = pe + 24;
   requirePeRange(bytes, optionalHeaderOffset, optionalSize);
   if (optionalSize < 64) peFailure("Truncated PE optional header.");
@@ -105,7 +111,7 @@ export const readPeHeaders = (bytes: Buffer, fileSize = bytes.length) => {
   if (headerSize < table + sectionCount * 40 || headerSize > fileSize)
     peFailure("Invalid PE SizeOfHeaders.");
   return {
-    machine: bytes.readUInt16LE(pe + 4),
+    machine,
     format,
     sectionCount,
     optionalHeaderOffset,
diff --git a/src/windows/WindowsNativeLoader.ts b/src/windows/WindowsNativeLoader.ts
index f213f85039..b036e88b82 100644
--- a/src/windows/WindowsNativeLoader.ts
+++ b/src/windows/WindowsNativeLoader.ts
@@ -1,3 +1,4 @@
+import { readPeCoffHeader } from "../domain/peCoffHeader.js";
 import { createHash } from "node:crypto";
 import { readFileSync } from "node:fs";
 import { createRequire } from "node:module";
@@ -195,10 +196,12 @@ const assertWindowsX64Artifact = (bytes: Buffer): void => {
   if (
     offset > bytes.length - 26 ||
     bytes.toString("latin1", offset, offset + 4) !== "PE\0\0" ||
-    bytes.readUInt16LE(offset + 4) !== 0x8664 ||
     bytes.readUInt16LE(offset + 24) !== 0x20b
   )
     throw new Error("Windows native artifact is not a Windows x64 PE32+ image");
+  const coff = readPeCoffHeader(bytes.subarray(offset + 4, offset + 24));
+  if (!coff.ok || coff.value.machine !== 0x8664)
+    throw new Error("Windows native artifact is not a Windows x64 PE32+ image");
 };
 
 let cached: WindowsNativeLoadResult | undefined;