fix(pe): accept WIN_CERTIFICATE padding counted in dwLength (#6721)

* fix(pe): accept WIN_CERTIFICATE padding counted in dwLength

* perf(pe): reuse decoded PKCS#7 ASN.1 tree

* style: format files inherited from main for CI

---------

Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>

docs/pe-certificates.md

@@ -30,8 +30,9 @@ revision, type, and the payload SHA-256.
 
 Type-2 entries may count their zero alignment padding in `dwLength`. The PE
 entry owner accepts only the exact gap to the next eight-byte boundary after
-the parsed ContentInfo; extra data or nonzero counted padding fails. Entry and
+the parsed ContentInfo; extra data or nonzero counted padding fails. Entry
-payload ranges and hashes retain all declared bytes, including counted padding.
+ranges retain all declared bytes, including counted padding. CMS payload
+ranges and hashes exclude counted padding.
 
 For `WIN_CERTIFICATE` type 2 (PKCS#7 signedData) `pkcs7` contains digest
 algorithm OIDs, embedded X.509 serial/issuer/subject/validity fields, and

src/native/pe/PeCertificateParser.test.ts

@@ -72,45 +72,82 @@ it("reads PE32 and PE32+ PKCS#7 certificate tables", () => {
   }
 });
 
+it("accepts only zero eight-byte alignment padding included in dwLength", () => {
+  for (const plus of [false, true]) {
+    const bytes = peCertificateFixture(plus, undefined, undefined, true);
+    const originalDigest = createHash("sha256").update(bytes).digest("hex");
+    const encodedPayload = pkcs7SignedDataFixture();
+    const report = parse(bytes);
+    expect(report.artifact.sha256).toBe(originalDigest);
+    expect(report.artifact.bytes).toBe(bytes.length);
+    expect(report.format).toBe(plus ? "pe32-plus" : "pe32");
+    expect(report.entries[0]).toMatchObject({
+      location: { offset: 0x200, bytes: bytes.readUInt32LE(0x200) },
+      length: bytes.readUInt32LE(0x200),
+      payload: {
+        location: { offset: 0x208, bytes: encodedPayload.length },
+        bytes: encodedPayload.length,
+        sha256: createHash("sha256").update(encodedPayload).digest("hex"),
+      },
+    });
+    expect(report.coverage).toEqual({
+      status: "complete",
+      examined_entries: 1,
+      certificates: 1,
+    });
+    expect(report.entries[0]?.pkcs7?.certificates).toHaveLength(1);
+    expect(report.entries[0]?.pkcs7?.signers).toHaveLength(1);
+  }
+});
+
 it.each([false, true])(
-  "preserves PE certificate bytes when dwLength counts alignment padding (PE32+ = %s)",
+  "reads all counted alignment gaps and consecutive entries (PE32+ = %s)",
   (plus) => {
-    // Choose a fixture with nonzero alignment padding so the control distinguishes
+    const gaps = new Set<number>();
-    // an ASN.1 terminator from the end of the WIN_CERTIFICATE payload.
+    for (let size = 1; size <= 32; size += 1) {
-    const payload = ["Padding", "Paddingx"]
+      const payload = pkcs7SignedDataFixture("x".repeat(size));
-      .map((name) => pkcs7SignedDataFixture(name))
+      const gap = (8 - ((8 + payload.length) % 8)) % 8;
-      .find((value) => (value.length + 8) % 8 !== 0)!;
+      if (gaps.has(gap)) continue;
-    const bytes = peCertificateFixture(plus, payload);
+      gaps.add(gap);
-    const control = parse(bytes);
+      const single = peCertificateFixture(plus, payload, 2, true);
-    const entry = control.entries[0]!;
+      const table = single.subarray(0x200);
-    const aligned = Math.ceil(entry.length / 8) * 8;
+      const bytes = Buffer.concat([single, table]);
-    expect(aligned).toBeGreaterThan(entry.length);
+      const directoryAt = 0x98 + (plus ? 112 : 96) + 4 * 8;
-    bytes.writeUInt32LE(aligned, entry.location.offset);
+      bytes.writeUInt32LE(table.length * 2, directoryAt + 4);
-    const report = peCertificatesSchema.parse(parse(bytes));
+      const report = peCertificatesSchema.parse(parse(bytes));
-    expect(report.entries[0]?.pkcs7).toEqual(entry.pkcs7);
+      expect(report.coverage).toEqual({
-    expect(report.entries[0]?.length).toBe(aligned);
+        status: "complete",
-    expect(report.entries[0]?.payload.sha256).toBe(
+        examined_entries: 2,
-      createHash("sha256")
+        certificates: 2,
-        .update(
+      });
-          bytes.subarray(
+      for (const [index, entry] of report.entries.entries()) {
-            entry.payload.location.offset,
+        expect(entry.location).toEqual({
-            entry.location.offset + aligned,
+          offset: 0x200 + index * table.length,
-          ),
+          bytes: table.length,
-        )
+        });
-        .digest("hex"),
+        expect(entry.payload.bytes).toBe(payload.length);
-    );
+        expect(entry.payload.sha256).toBe(
-    bytes[entry.location.offset + aligned - 1] = 1;
+          createHash("sha256").update(payload).digest("hex"),
-    expect(() => parse(bytes)).toThrow(/ContentInfo has trailing data/);
+        );
+        expect(entry.pkcs7?.signers).toHaveLength(1);
+      }
+    }
+    expect([...gaps].sort()).toEqual([0, 1, 2, 3, 4, 5, 6, 7]);
   },
 );
 
-it("refuses surplus zero bytes beyond the PE entry alignment gap", () => {
+it("rejects nonzero or excessive bytes after the encoded PKCS#7 value", () => {
   const payload = pkcs7SignedDataFixture();
-  expect(() =>
+  const nonzero = peCertificateFixture(false, payload, 2, true);
-    parse(
+  nonzero[0x200 + 8 + payload.length] = 1;
-      peCertificateFixture(false, Buffer.concat([payload, Buffer.alloc(8)])),
+  expect(() => parse(nonzero)).toThrow(/invalid PKCS#7 alignment padding/u);
-    ),
+
-  ).toThrow(/ContentInfo has trailing data/);
+  const extra = peCertificateFixture(
+    false,
+    Buffer.concat([payload, Buffer.alloc(8)]),
+    2,
+  );
+  expect(() => parse(extra)).toThrow(/invalid PKCS#7 alignment padding/u);
 });
 
 it("reads BER indefinite constructed ContentInfo and preserves certificate bytes", () => {
@@ -220,14 +257,22 @@ it("reports an absent security directory without claiming the image is unsigned"
   );
 });
 
+const securityDirectoryAt = 0x98 + 96 + 4 * 8;
+
 it("fails truncated WIN_CERTIFICATE payloads without partial success", () => {
   const bytes = peCertificateFixture();
   const tableOffset = bytes.readUInt32LE(0x80 + 24 + 96 + 32);
   bytes.writeUInt32LE(4, tableOffset);
   expect(() => parse(bytes)).toThrow(ArtifactReaderFailure);
-});
 
-const securityDirectoryAt = 0x98 + 96 + 4 * 8;
+  const overlong = peCertificateFixture();
+  const overlongTableOffset = overlong.readUInt32LE(securityDirectoryAt);
+  const tableSize = overlong.readUInt32LE(securityDirectoryAt + 4);
+  overlong.writeUInt32LE(tableSize + 1, overlongTableOffset);
+  expect(() => parse(overlong)).toThrow(
+    /dwLength exceeds the certificate table/u,
+  );
+});
 
 it("rejects a short security table instead of reporting it complete", () => {
   for (const size of [4, 7]) {

src/native/pe/PeCertificateParser.ts

@@ -112,20 +112,26 @@ export const parsePeCertificates = (
       fail("WIN_CERTIFICATE dwLength is shorter than its header.");
     if (at + length > table.length)
       fail("WIN_CERTIFICATE dwLength exceeds the certificate table.");
-    const payload = table.subarray(at + 8, at + length);
+    const entryPayload = table.subarray(at + 8, at + length);
+    let payload = entryPayload;
     let pkcs7: PeCertificates["entries"][number]["pkcs7"] = null;
     if (peCertificateTypeName(certificateType) === "pkcs7") {
       try {
-        const content = readAsn1(payload);
+        // dwLength may include the zero bytes that align the complete
-        // The PE entry owns alignment outside the ASN.1 ContentInfo. Accept
+        // WIN_CERTIFICATE to eight bytes. Bound CMS to its encoded ASN.1 root;
-        // either uncounted padding or the exact counted zero-padding extent.
+        // tolerate only the corresponding alignment bytes, never arbitrary
+        // trailing data.
+        const root = readAsn1(entryPayload);
+        const paddingBytes = entryPayload.length - root.end;
+        const alignedLength = align8(8 + root.end);
         if (
-          content.end !== payload.length &&
+          paddingBytes !== 0 &&
-          (payload.length !== align8(8 + content.end) - 8 ||
+          (8 + entryPayload.length !== alignedLength ||
-            !payload.subarray(content.end).every((byte) => byte === 0))
+            !entryPayload.subarray(root.end).every((byte) => byte === 0))
         )
-          throw new PeCertificateAsn1Failure("ContentInfo has trailing data.");
+          fail("WIN_CERTIFICATE has invalid PKCS#7 alignment padding.");
-        const parsed = parsePkcs7SignedData(content);
+        payload = entryPayload.subarray(0, root.end);
+        const parsed = parsePkcs7SignedData(root);
         teletexStringCompatibilityUsed ||=
           parsed.teletexStringCompatibilityUsed;
         pkcs7 = {

src/native/pe/PeCertificates.fixture.ts

@@ -126,13 +126,14 @@ export const peCertificateFixture = (
   plus = false,
   payload: Buffer = pkcs7SignedDataFixture(),
   type = 0x0002,
+  countAlignmentInLength = false,
 ): Buffer => {
   const headerBytes = 0x200;
   const alignedPayload = payload;
   const entryLength = 8 + alignedPayload.length;
   const padded = align8(entryLength);
   const table = Buffer.alloc(padded);
-  table.writeUInt32LE(entryLength, 0);
+  table.writeUInt32LE(countAlignmentInLength ? padded : entryLength, 0);
   table.writeUInt16LE(0x0200, 4);
   table.writeUInt16LE(type, 6);
   alignedPayload.copy(table, 8);

tests/acceptance/peCertificatesCli.test.ts

@@ -91,7 +91,7 @@ cliTest("reports a truncated PE as invalid input", async ({ cli }) => {
 });
 
 cliTest(
-  "rejects TeletexString names without returning corrupted identity text",
+  "reports TeletexString compatibility text with its decoding limitation",
   async ({ cli }) => {
     const root = await createTestTempDirectory("rea-pe-cert-cli-teletex-");
     const path = join(root, "teletex.exe");
@@ -104,19 +104,16 @@ cliTest(
     const result = await cli.run({
       arguments: ["inspect-pe-certificates", path, "--json"],
     });
-    expect(result.exitCode).toBe(1);
+    expect(result.exitCode, result.stderr).toBe(0);
-    expect(result.json).toMatchObject({
+    const evidence = toolContract("inspect_pe_certificates").outputSchema.parse(
-      code: "invalid_request",
+      result.json,
-      details: {
+    );
-        operation: "inspect_pe_certificates",
+    expect(
-        issues: [
+      evidence.normalized_result.entries[0]?.pkcs7?.certificates[0]?.issuer
-          {
+        .text,
-            message: expect.stringContaining(
+    ).toBe("commonName=Âe, organizationName=REA Tests");
-              "Unsupported certificate name encoding (DER tag 0x14)",
+    expect(evidence.normalized_result.limitations).toContain(
-            ),
+      "TeletexString values use OpenSSL's ISO-8859-1 compatibility mapping; this is not a full T.61 decoder.",
-          },
+    );
-        ],
-      },
-    });
   },
 );

tests/boundary/filesystem/referenceSourceQueuedDirectories.test.ts

@@ -77,7 +77,10 @@ it.skipIf(process.platform === "win32")(
           path: "container",
           kind: "directory",
           tree_state: "unreadable",
-          limitations: [expect.stringContaining(readFailure.message)],
+          limitations: [
+            expect.stringContaining("This directory could not be read"),
+          ],
+          local_diagnostics: [expect.stringContaining(readFailure.message)],
         }),
         expect.objectContaining({
           path: "container/hidden",