From ab30df08f7a83bc3778c328e71b09a9cbf604a7d Mon Sep 17 00:00:00 2001 From: Dhruvilkumar Ashokbhai Chodvadiya <50516925+dhruvac29@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:54:20 -0700 Subject: [PATCH] fix(targets): keep original DOS header padding out of e_lfanew (#7013) * fix(targets): keep original DOS padding out of e_lfanew A complete DOS header with a nonempty relocation table that ended before e_lfanew treated the padding dword at 0x3c as a Windows declaration. Without a recognized Windows signature, fall back to the validated DOS structure so original DOS images are not refused as malformed Windows targets. Preserve unresolved-candidate handling when no relocation table delimits the padding. * test(ghidra): cover DOS padding and unresolved header candidates * fix(targets): read Windows signatures before dismissing DOS padding * docs(targets): reconcile DOS padding admission contract Document that relocation-table padding inside a complete DOS header is DOS data, while an unread Windows signature beyond the current probe is requested before that rule applies. Aligns docs/installation.md with docs/ghidra-dos.md after the MZ admission fix. --------- Co-authored-by: dhruvac29 --- docs/ghidra-dos.md | 19 +++-- docs/installation.md | 10 ++- .../ArtifactGraphConstruction.test.ts | 6 +- src/domain/dosMz.test.ts | 69 ++++++++++++++++--- src/domain/dosMz.ts | 14 +++- src/ghidra/GhidraLoadImageValues.test.ts | 27 +++++++- src/native/metadata/PeImage.test.ts | 4 +- .../boundary/filesystem/binaryTarget.test.ts | 54 +++++++++------ 8 files changed, 156 insertions(+), 47 deletions(-) diff --git a/docs/ghidra-dos.md b/docs/ghidra-dos.md index 8f7b8a98d8..e2fdd94f64 100644 --- a/docs/ghidra-dos.md +++ b/docs/ghidra-dos.md @@ -29,13 +29,18 @@ point using the available structural bytes and actual file length. It reports those DOS facts separately from Windows-header evidence. A recognized PE signature remains PE; recognized NE, LE, and LX headers are explicitly unsupported. A nonzero Windows-header pointer without a recognized signature -remains an unresolved candidate, even when the DOS structure validates: -automatic target selection fails closed and reports both observations. -REA does not infer DOS intent from padding or infer a damaged Windows header -from the pointer alone. Inventory classification is a bounded-prefix format -hint; it returns `unknown` for that candidate or when its prefix cannot -establish the format. That result is distinct from complete DOS validation and -does not decide target admission. +usually remains an unresolved candidate, even when the DOS structure validates: +automatic target selection fails closed and reports both observations. When a +nonempty relocation table ends inside a complete DOS header, the remaining +header bytes belong to DOS padding; a nonzero dword there is not a Windows +declaration. An unread signature beyond the current probe is requested before +that padding rule applies. This avoids manufacturing a Windows header from +original DOS padding while preserving rejection of damaged Windows +declarations. +Inventory classification is a bounded-prefix format hint; it returns `unknown` +for an unresolved candidate or when its prefix cannot establish the format. +That result is distinct from complete DOS validation and does not decide target +admission. Ghidra imports a private target snapshot with `MzLoader`, language `x86:LE:16:Real Mode`, and compiler specification `default`. These settings, diff --git a/docs/installation.md b/docs/installation.md index a3f60e6bb3..20743f95f0 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -997,9 +997,13 @@ Target admission preserves recognized PE/NE/LE/LX signatures separately from reader layout support. A Windows header overlapping the first 64 DOS-header bytes remains an unsupported layout in this release; it is not reported as an invalid offset. Tiny PE files with short optional headers have additional -unverified loader requirements. Unconfirmed nonzero DOS padding remains an -unresolved Windows-header candidate, with independently validated DOS facts in -the target error diagnostics; those facts do not prove a Windows header absent. +unverified loader requirements. A nonzero Windows-header pointer without a +recognized signature remains an unresolved candidate, with independently +validated DOS facts in the target error diagnostics; those facts do not prove a +Windows header absent. When a nonempty relocation table ends inside a complete +DOS header, the remaining header bytes are DOS padding, so a nonzero dword +there is not a Windows declaration. An unread signature beyond the current +probe is requested before that padding rule applies. On Linux and macOS, `annotate_native_function` also edits a function name and/or entry comments atomically and returns refreshed analysis, and `set_address_name` and `set_addresses_names` name a function entry or any other mapped address diff --git a/src/artifacts/inventory/ArtifactGraphConstruction.test.ts b/src/artifacts/inventory/ArtifactGraphConstruction.test.ts index ee55055610..375c31a460 100644 --- a/src/artifacts/inventory/ArtifactGraphConstruction.test.ts +++ b/src/artifacts/inventory/ArtifactGraphConstruction.test.ts @@ -44,7 +44,7 @@ it("joins only observed PE signature bytes across stream chunks", () => { ).toBe("pe"); }); -it("keeps an ambiguous DOS MZ inventory candidate unknown", () => { +it("classifies original DOS header padding as an executable", () => { const bytes = Buffer.alloc(128); bytes.write("MZ", 0, "ascii"); bytes.writeUInt16LE(128, 2); @@ -54,8 +54,8 @@ it("keeps an ambiguous DOS MZ inventory candidate unknown", () => { bytes.writeUInt16LE(28, 24); bytes.write("ABCD", 60, "ascii"); expect(classifyArtifactBytesAtPath("legacy.exe", bytes)).toEqual({ - kind: "unknown", - format: "unknown", + kind: "executable", + format: "dos-mz", }); }); diff --git a/src/domain/dosMz.test.ts b/src/domain/dosMz.test.ts index 3e7b874926..00f444430a 100644 --- a/src/domain/dosMz.test.ts +++ b/src/domain/dosMz.test.ts @@ -284,8 +284,10 @@ describe("MZ Windows header observations", () => { }), ).toEqual({ state: "declared", offset, signature: "PE" }); }); +}); - it("preserves DOS facts while leaving an unsigned Windows pointer unresolved", () => { +describe("MZ header padding observations", () => { + it("treats a complete header after the relocation table as DOS padding", () => { const bytes = Buffer.alloc(128); bytes.write("MZ", 0, "ascii"); bytes.writeUInt16LE(128, 2); @@ -308,15 +310,66 @@ describe("MZ Windows header observations", () => { relocationTableOffset: 28, }, }); - expect(evidence.windowsHeader).toEqual({ + expect(evidence.windowsHeader).toEqual({ state: "absent" }); + expect(parseExecutableHeader(bytes, "x64")).toMatchObject({ + ok: true, + value: { format: "dos-mz", architecture: "x86" }, + }); + + bytes.writeUInt32LE(0xfffffff0, 60); + expect(parseMzHeaderEvidence(bytes).windowsHeader).toEqual({ + state: "absent", + }); + expect(parseExecutableHeader(bytes, "x64")).toMatchObject({ + ok: true, + value: { format: "dos-mz", architecture: "x86" }, + }); + }); + + it("requests an unread signature before treating padding as DOS data", () => { + const offset = 8192; + const bytes = Buffer.alloc(offset + 512); + bytes.write("MZ", 0, "ascii"); + bytes.writeUInt16LE(128, 2); + bytes.writeUInt16LE(1, 4); + bytes.writeUInt16LE(2, 6); + bytes.writeUInt16LE(4, 8); + bytes.writeUInt16LE(28, 24); + bytes.writeUInt32LE(offset, 60); + const prefix = bytes.subarray(0, 4096); + + expect(observeMzWindowsHeader(prefix, bytes.length)).toEqual({ + state: "signature-needed", + offset, + }); + expect( + observeMzWindowsHeader(prefix, bytes.length, { + offset, + bytes: Buffer.from("PE\0\0", "binary"), + }), + ).toEqual({ state: "declared", offset, signature: "PE" }); + expect( + observeMzWindowsHeader(prefix, bytes.length, { + offset, + bytes: Buffer.from("ZZZZ", "ascii"), + }), + ).toEqual({ state: "absent" }); + }); + + it("keeps an unconfirmed pointer outside a relocation table unresolved", () => { + const bytes = Buffer.alloc(256); + dosMz().copy(bytes, 0, 0, 28); + bytes.writeUInt16LE(256, 2); + bytes.writeUInt16LE(4, 8); + bytes.writeUInt16LE(64, 24); + bytes.writeUInt32LE(192, 60); + + expect(parseMzHeaderEvidence(bytes).windowsHeader).toEqual({ state: "candidate", - offset: 0x44434241, - signature: "unobserved", - }); - expect(parseExecutableHeader(bytes, "x64")).toEqual({ - ok: false, - error: "MZ Windows new-header candidate is unresolved", + offset: 192, + signature: "other", }); + expect(parseExecutableHeader(bytes, "x64")).toMatchObject({ ok: false }); bytes.writeUInt32LE(0xfffffff0, 60); expect(parseMzHeaderEvidence(bytes).windowsHeader).toEqual({ diff --git a/src/domain/dosMz.ts b/src/domain/dosMz.ts index 02ed5076f2..870e315c5c 100644 --- a/src/domain/dosMz.ts +++ b/src/domain/dosMz.ts @@ -72,11 +72,12 @@ export const observeMzWindowsHeader = ( const headerBytes = bytes.readUInt16LE(8) * 16; const relocationCount = bytes.readUInt16LE(6); const tableStart = bytes.readUInt16LE(24); + const tableEnd = tableStart + relocationCount * 4; if ( headerBytes !== 0 && relocationCount > 0 && tableStart < 64 && - tableStart + relocationCount * 4 > 60 + tableEnd > 60 ) return { state: "absent" }; const offset = bytes.readUInt32LE(60); @@ -92,6 +93,17 @@ export const observeMzWindowsHeader = ( const signature = windowsHeaderSignature(available); if (signature !== "other" && signature !== "unobserved") return { state: "declared", offset, signature }; + // A relocation table that ends inside a complete DOS header owns the rest of + // that header as padding. An observed non-Windows signature confirms that a + // dword in that padding is DOS data. An unread signature still requests the + // bounded follow-up read when enough bytes remain to recognize one. + if ( + headerBytes >= 64 && + relocationCount > 0 && + tableEnd <= 60 && + (signature === "other" || fileSize - offset < 2) + ) + return { state: "absent" }; if (headerBytes === 0 || headerBytes >= 64) { const remaining = fileSize - offset; const needsMore = diff --git a/src/ghidra/GhidraLoadImageValues.test.ts b/src/ghidra/GhidraLoadImageValues.test.ts index 249fc49421..4d99667c54 100644 --- a/src/ghidra/GhidraLoadImageValues.test.ts +++ b/src/ghidra/GhidraLoadImageValues.test.ts @@ -13,7 +13,7 @@ import type { NativeLoadImageObservation } from "../domain/native/nativeLoadImag import { parseGhidraInventoryResult } from "./GhidraInventoryValues.js"; import { prepareGhidraDosLoadImage } from "./GhidraLoadImageValues.js"; -it("does not interpret an ambiguous Windows-header candidate as a DOS image", () => { +it("accepts DOS header padding after a complete relocation table", () => { const bytes = Buffer.alloc(128); bytes.write("MZ", 0, "ascii"); bytes.writeUInt16LE(128, 2); @@ -22,6 +22,31 @@ it("does not interpret an ambiguous Windows-header candidate as a DOS image", () bytes.writeUInt16LE(4, 8); bytes.writeUInt16LE(28, 24); bytes.write("ABCD", 60, "ascii"); + expect(prepareGhidraDosLoadImage(bytes, bytes.length)).toMatchObject({ + ok: true, + value: { + header: { + headerBytes: 64, + imageBytes: 128, + moduleBytes: 64, + overlayBytes: 0, + relocationCount: 2, + relocationTableOffset: 28, + }, + loadSegment: 0x1000, + }, + }); +}); + +it("does not interpret an unresolved Windows-header candidate as a DOS image", () => { + const bytes = Buffer.alloc(128); + bytes.write("MZ", 0, "ascii"); + bytes.writeUInt16LE(128, 2); + bytes.writeUInt16LE(1, 4); + bytes.writeUInt16LE(0, 6); + bytes.writeUInt16LE(4, 8); + bytes.writeUInt16LE(28, 24); + bytes.write("ABCD", 60, "ascii"); expect(prepareGhidraDosLoadImage(bytes, bytes.length)).toEqual({ ok: false, error: "MZ Windows new-header candidate is unresolved", diff --git a/src/native/metadata/PeImage.test.ts b/src/native/metadata/PeImage.test.ts index 99fc6b7934..44fcfcc2a7 100644 --- a/src/native/metadata/PeImage.test.ts +++ b/src/native/metadata/PeImage.test.ts @@ -19,7 +19,7 @@ it("classifies a confirmed overlapping PE as an unsupported layout", () => { } }); -it("keeps a DOS-valid but unsigned Windows-header candidate unresolved", () => { +it("reports complete DOS MZ images as outside the PE reader", () => { const bytes = Buffer.alloc(128); bytes.write("MZ", 0, "ascii"); bytes.writeUInt16LE(128, 2); @@ -29,7 +29,7 @@ it("keeps a DOS-valid but unsigned Windows-header candidate unresolved", () => { bytes.writeUInt16LE(28, 24); bytes.write("ABCD", 60, "ascii"); expect(() => readPeImage(bytes)).toThrow( - "MZ Windows new-header candidate is unresolved", + "DOS MZ images are outside the supported ELF, PE and thin Mach-O formats", ); }); diff --git a/tests/boundary/filesystem/binaryTarget.test.ts b/tests/boundary/filesystem/binaryTarget.test.ts index 0a417aea13..efafa4adc9 100644 --- a/tests/boundary/filesystem/binaryTarget.test.ts +++ b/tests/boundary/filesystem/binaryTarget.test.ts @@ -1219,8 +1219,8 @@ describe("DOS binary target I/O", () => { }); }); - it("retains valid DOS facts while refusing automatic selection for an unresolved pointer", async () => { - const directory = await createTestTempDirectory("rea-ambiguous-mz-target-"); + it("admits original DOS header padding after a relocation table", async () => { + const directory = await createTestTempDirectory("rea-dos-header-padding-"); const path = join(directory, "legacy.exe"); const bytes = Buffer.alloc(128); bytes.write("MZ", 0, "ascii"); @@ -1232,26 +1232,36 @@ describe("DOS binary target I/O", () => { bytes.write("ABCD", 60, "ascii"); await writeFile(path, bytes); - const result = await parseBinaryTarget(path); - expect(result).toMatchObject({ - ok: false, - error: { - reason: "MZ Windows new-header candidate is unresolved", - diagnostics: { - mz_windows_header: { - state: "candidate", - offset: 0x44434241, - signature: "unobserved", - }, - mz_dos: { - state: "valid", - header_bytes: 64, - image_bytes: 128, - module_bytes: 64, - relocation_count: 2, - relocation_table_offset: 28, - }, - }, + expect(await parseBinaryTarget(path)).toMatchObject({ + ok: true, + value: { format: "dos-mz", architecture: "x86", kind: "executable" }, + }); + }); + + it("resolves a PE signature beyond the prefix when DOS padding follows a relocation table", async () => { + const directory = await createTestTempDirectory( + "rea-padded-stub-pe-target-", + ); + const path = join(directory, "padded-stub.dll"); + const offset = 8192; + const stub = Buffer.alloc(offset); + stub.write("MZ", 0, "ascii"); + stub.writeUInt16LE(offset % 512, 2); + stub.writeUInt16LE(Math.ceil(offset / 512), 4); + stub.writeUInt16LE(2, 6); + stub.writeUInt16LE(4, 8); + stub.writeUInt16LE(28, 24); + stub.writeUInt32LE(offset, 60); + const peRecord = pe(0x8664, offset, 0x2002).subarray(offset); + const bytes = Buffer.concat([stub, peRecord]); + await writeFile(path, bytes); + + expect(await parseBinaryTarget(path)).toMatchObject({ + ok: true, + value: { + format: "pe", + architecture: "x86_64", + executableRole: "shared-library", }, }); });