From 047d75a498f18a6608f35ee5e36cf1bf0f81ca97 Mon Sep 17 00:00:00 2001 From: ShuaixinHuang <1836656514@qq.com> Date: Thu, 12 Nov 2026 07:43:17 +0800 Subject: [PATCH] fix(pe): honor import directory bounds (#10044) Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- docs/pe-imports.md | 9 ++++-- src/native/pe/PeImportParser.test.ts | 43 ++++++++++++++++++++++++++++ src/native/pe/PeImportParser.ts | 24 ++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/docs/pe-imports.md b/docs/pe-imports.md index 5e0836e70d..ac710a9e22 100644 --- a/docs/pe-imports.md +++ b/docs/pe-imports.md @@ -33,7 +33,9 @@ Evidence file-offset range. If the first descriptor cannot be examined, the directory declaration remains present with no consumed `locations`, and coverage is partial. Descriptors, thunks and NUL-terminated names stay within PE's 32-bit RVA space, including -PE32+ images; earlier valid rows survive an overflowing descriptor walk. +PE32+ images. Descriptor walks also stay inside each data directory's declared +byte size; an unterminated or partially covered final slot is reported as a +malformed entry, while earlier valid rows survive. `machine` retains the original unsigned COFF value. `architecture` decodes known machines, including ReadyToRun OS overrides when a CLI directory is @@ -69,8 +71,9 @@ with no runtime override; `max_file_bytes`, `max_dlls` and `max_functions` do not raise it. Resource failures report the exhausted allowance as `limit` (`dlls`, `functions` or `retained_text`) with matching recovery. -Unreadable DLL names, hint/name entries, or thunks are reported as partial -coverage with `issues`. A descriptor whose required table RVA is zero (a +Unreadable DLL names, hint/name entries, thunks, or descriptor slots outside +the declared directory size are reported as partial coverage with `issues`. A +descriptor whose required table RVA is zero (a standard `FirstThunk`, or a delay-load import address or name table) is reported as `malformed_entry` with no functions, never as a complete empty inventory. Delay-load descriptors that store virtual addresses rather than RVAs diff --git a/src/native/pe/PeImportParser.test.ts b/src/native/pe/PeImportParser.test.ts index 23454da7b4..27eaf5ce0e 100644 --- a/src/native/pe/PeImportParser.test.ts +++ b/src/native/pe/PeImportParser.test.ts @@ -42,6 +42,49 @@ it("reads named PE32+ imports and a pefile-compatible import hash", async () => expect(report.delay_imports).toEqual([]); }); +it.each(["standard", "delay"] as const)( + "does not read %s descriptors past the declared directory size", + async (kind) => { + const bytes = peImportImage( + kind === "standard" + ? { + dlls: [ + { name: "ONE.dll", functions: [{ name: "First" }] }, + { name: "TWO.dll", functions: [{ name: "Second" }] }, + ], + } + : { + dlls: [], + delayDlls: [ + { name: "ONE.dll", functions: [{ name: "First" }] }, + { name: "TWO.dll", functions: [{ name: "Second" }] }, + ], + }, + ); + const directoryAt = 152 + 112 + (kind === "standard" ? 1 : 13) * 8; + const directoryRva = bytes.readUInt32LE(directoryAt); + const descriptorBytes = kind === "standard" ? 20 : 32; + bytes.writeUInt32LE(descriptorBytes, directoryAt + 4); + + const report = await parse(bytes); + const rows = kind === "standard" ? report.imports : report.delay_imports; + const directory = + kind === "standard" + ? report.import_directory + : report.delay_import_directory; + expect(rows.map((row) => row.name)).toEqual(["ONE.dll"]); + expect(directory?.locations).toHaveLength(1); + expect(report.coverage.status).toBe("partial"); + expect(report.issues).toContainEqual( + expect.objectContaining({ + code: "malformed_entry", + location: `0x${(directoryRva + descriptorBytes).toString(16)}`, + message: expect.stringContaining("ends before a null descriptor"), + }), + ); + }, +); + const parse = (bytes: Buffer) => parsePeImports(bytes, artifact(bytes), input); it.each([ diff --git a/src/native/pe/PeImportParser.ts b/src/native/pe/PeImportParser.ts index 83fc3a76a3..842144fc85 100644 --- a/src/native/pe/PeImportParser.ts +++ b/src/native/pe/PeImportParser.ts @@ -395,6 +395,18 @@ export const parsePeImports = async ( await checkpoint(); const descriptorRva = importDirectory.address + index * IMPORT_DESCRIPTOR_BYTES; + if ( + index * IMPORT_DESCRIPTOR_BYTES + IMPORT_DESCRIPTOR_BYTES > + importDirectory.size + ) { + issues.push({ + code: "malformed_entry", + location: hex(descriptorRva), + message: + "Import directory ends before a null descriptor; its declared size does not cover the complete descriptor slot", + }); + break; + } const descriptor = mapDescriptor( descriptorRva, IMPORT_DESCRIPTOR_BYTES, @@ -469,6 +481,18 @@ export const parsePeImports = async ( await checkpoint(); const descriptorRva = delayDirectory.address + index * DELAY_DESCRIPTOR_BYTES; + if ( + index * DELAY_DESCRIPTOR_BYTES + DELAY_DESCRIPTOR_BYTES > + delayDirectory.size + ) { + issues.push({ + code: "malformed_entry", + location: hex(descriptorRva), + message: + "Delay-load directory ends before a null descriptor; its declared size does not cover the complete descriptor slot", + }); + break; + } const descriptor = mapDescriptor( descriptorRva, DELAY_DESCRIPTOR_BYTES,