diff --git a/docs/pe-certificates.md b/docs/pe-certificates.md index 0987cee27d..017318f0cd 100644 --- a/docs/pe-certificates.md +++ b/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 -payload ranges and hashes retain all declared bytes, including counted padding. +the parsed ContentInfo; extra data or nonzero counted padding fails. Entry +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 diff --git a/src/native/pe/PeCertificateParser.test.ts b/src/native/pe/PeCertificateParser.test.ts index 4e94bf88e0..f88dde7e36 100644 --- a/src/native/pe/PeCertificateParser.test.ts +++ b/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 - // an ASN.1 terminator from the end of the WIN_CERTIFICATE payload. - const payload = ["Padding", "Paddingx"] - .map((name) => pkcs7SignedDataFixture(name)) - .find((value) => (value.length + 8) % 8 !== 0)!; - const bytes = peCertificateFixture(plus, payload); - const control = parse(bytes); - const entry = control.entries[0]!; - const aligned = Math.ceil(entry.length / 8) * 8; - expect(aligned).toBeGreaterThan(entry.length); - bytes.writeUInt32LE(aligned, entry.location.offset); - const report = peCertificatesSchema.parse(parse(bytes)); - expect(report.entries[0]?.pkcs7).toEqual(entry.pkcs7); - expect(report.entries[0]?.length).toBe(aligned); - expect(report.entries[0]?.payload.sha256).toBe( - createHash("sha256") - .update( - bytes.subarray( - entry.payload.location.offset, - entry.location.offset + aligned, - ), - ) - .digest("hex"), - ); - bytes[entry.location.offset + aligned - 1] = 1; - expect(() => parse(bytes)).toThrow(/ContentInfo has trailing data/); + const gaps = new Set(); + for (let size = 1; size <= 32; size += 1) { + const payload = pkcs7SignedDataFixture("x".repeat(size)); + const gap = (8 - ((8 + payload.length) % 8)) % 8; + if (gaps.has(gap)) continue; + gaps.add(gap); + const single = peCertificateFixture(plus, payload, 2, true); + const table = single.subarray(0x200); + const bytes = Buffer.concat([single, table]); + const directoryAt = 0x98 + (plus ? 112 : 96) + 4 * 8; + bytes.writeUInt32LE(table.length * 2, directoryAt + 4); + const report = peCertificatesSchema.parse(parse(bytes)); + expect(report.coverage).toEqual({ + status: "complete", + examined_entries: 2, + certificates: 2, + }); + for (const [index, entry] of report.entries.entries()) { + expect(entry.location).toEqual({ + offset: 0x200 + index * table.length, + bytes: table.length, + }); + expect(entry.payload.bytes).toBe(payload.length); + expect(entry.payload.sha256).toBe( + createHash("sha256").update(payload).digest("hex"), + ); + 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(() => - parse( - peCertificateFixture(false, Buffer.concat([payload, Buffer.alloc(8)])), - ), - ).toThrow(/ContentInfo has trailing data/); + const nonzero = peCertificateFixture(false, payload, 2, true); + nonzero[0x200 + 8 + payload.length] = 1; + expect(() => parse(nonzero)).toThrow(/invalid PKCS#7 alignment padding/u); + + 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]) { diff --git a/src/native/pe/PeCertificateParser.ts b/src/native/pe/PeCertificateParser.ts index 9866a9ee76..4b55e2f61e 100644 --- a/src/native/pe/PeCertificateParser.ts +++ b/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); - // The PE entry owns alignment outside the ASN.1 ContentInfo. Accept - // either uncounted padding or the exact counted zero-padding extent. + // dwLength may include the zero bytes that align the complete + // WIN_CERTIFICATE to eight bytes. Bound CMS to its encoded ASN.1 root; + // 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 && - (payload.length !== align8(8 + content.end) - 8 || - !payload.subarray(content.end).every((byte) => byte === 0)) + paddingBytes !== 0 && + (8 + entryPayload.length !== alignedLength || + !entryPayload.subarray(root.end).every((byte) => byte === 0)) ) - throw new PeCertificateAsn1Failure("ContentInfo has trailing data."); - const parsed = parsePkcs7SignedData(content); + fail("WIN_CERTIFICATE has invalid PKCS#7 alignment padding."); + payload = entryPayload.subarray(0, root.end); + const parsed = parsePkcs7SignedData(root); teletexStringCompatibilityUsed ||= parsed.teletexStringCompatibilityUsed; pkcs7 = { diff --git a/src/native/pe/PeCertificates.fixture.ts b/src/native/pe/PeCertificates.fixture.ts index 0960d675ea..05feaa4ef1 100644 --- a/src/native/pe/PeCertificates.fixture.ts +++ b/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); diff --git a/tests/acceptance/peCertificatesCli.test.ts b/tests/acceptance/peCertificatesCli.test.ts index 8b4a7105fe..df63000084 100644 --- a/tests/acceptance/peCertificatesCli.test.ts +++ b/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.json).toMatchObject({ - code: "invalid_request", - details: { - operation: "inspect_pe_certificates", - issues: [ - { - message: expect.stringContaining( - "Unsupported certificate name encoding (DER tag 0x14)", - ), - }, - ], - }, - }); + expect(result.exitCode, result.stderr).toBe(0); + const evidence = toolContract("inspect_pe_certificates").outputSchema.parse( + result.json, + ); + expect( + evidence.normalized_result.entries[0]?.pkcs7?.certificates[0]?.issuer + .text, + ).toBe("commonName=Âe, organizationName=REA Tests"); + expect(evidence.normalized_result.limitations).toContain( + "TeletexString values use OpenSSL's ISO-8859-1 compatibility mapping; this is not a full T.61 decoder.", + ); }, ); diff --git a/tests/boundary/filesystem/referenceSourceQueuedDirectories.test.ts b/tests/boundary/filesystem/referenceSourceQueuedDirectories.test.ts index c5df498bc9..3fb77dae40 100644 --- a/tests/boundary/filesystem/referenceSourceQueuedDirectories.test.ts +++ b/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",