From 7857d2d32960424512f238236d340de2c503bb99 Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Wed, 7 Oct 2026 12:50:52 +0000 Subject: [PATCH] fix(artifacts): apply the integrity policy to xar checksums and bound symlinks - Inventory scans now verify xar extracted checksums themselves, under the caller's integrity policy. Under "fail" a mismatch still fails, now as artifact_integrity_mismatch with the logical path. Under "record-and-continue" the member stays in the graph as mismatched, with the declared and observed values, and is not expanded. A declared SHA-256 becomes an integrity contradiction. Extraction keeps verifying in the reader. - Read cpio symlink targets only up to 4096 bytes; longer declared targets are streamed past and recorded with a limitation instead of buffered. Co-Authored-By: Claude Opus 5.5 --- .../managed-conformance-manifest.json | 4 +- .../references/native-and-artifacts.md | 5 +- src/application/ArtifactInventory/reader.ts | 6 +- .../ArtifactInventory/scanCanonical.ts | 41 +++++++++---- .../ArtifactInventory/scanReader.ts | 57 ++++++++++++------- src/artifacts/ArtifactHash.ts | 13 ++++- src/artifacts/ArtifactReader.ts | 9 +++ src/artifacts/CpioArtifactReader.ts | 14 ++++- src/artifacts/InstallerPackageReaders.test.ts | 33 +++++++++++ src/artifacts/XarArtifactReader.ts | 35 ++++++++++-- .../installerPackageInventory.test.ts | 38 ++++++++++++- 11 files changed, 212 insertions(+), 43 deletions(-) diff --git a/docs/verification/managed-conformance-manifest.json b/docs/verification/managed-conformance-manifest.json index 20f7a0cbf0..b8d5ca2f85 100644 --- a/docs/verification/managed-conformance-manifest.json +++ b/docs/verification/managed-conformance-manifest.json @@ -1,5 +1,5 @@ { - "manifest_id": "ecm_9b3d014e67d8c2ccc8736694d59b38acf6d8ab15d1e9cdeb3998339ff33b5cc2", + "manifest_id": "ecm_adf2c0c73407ba5b57cbfda24a2d7e02b876657936ff7cfff9e3136b51e17f29", "verifier": { "id": "rea-managed-conformance", "version": "1" @@ -13,7 +13,7 @@ "skill_digests": [ { "skill_id": "reverse-engineer-anything", - "sha256": "17120894d8defd8c28791e344b9241e8d3abcba5ac345b0c8cca92adda518dfd" + "sha256": "92ab55fc457234c889e2f8a7043accc732ca9f6a253415c1d9758ffa9c4160ee" } ], "claims": [ diff --git a/skills/reverse-engineer-anything/references/native-and-artifacts.md b/skills/reverse-engineer-anything/references/native-and-artifacts.md index bb46b23e03..6170d837ae 100644 --- a/skills/reverse-engineer-anything/references/native-and-artifacts.md +++ b/skills/reverse-engineer-anything/references/native-and-artifacts.md @@ -67,7 +67,10 @@ header, depending on the binary and analyzer version. Use `inspect_artifact` for application bundles, archives, ZIP/APK/IPA/MSIX/AppX, ASAR, DMG, or PKG inputs. A PKG inventory includes Distribution, PackageInfo, the pre/postinstall scripts, and the gzip payload's files. Pbzx payloads are -recorded only. Inspect it when the artifact graph and findings help answer the +recorded only. Checksums declared in the PKG's table of contents are verified; +with `integrity_policy: "record-and-continue"`, a mismatched member stays in +the graph as `mismatched`, with the declared and observed values, and is not +expanded. Inspect it when the artifact graph and findings help answer the question. It returns the complete artifact graph inline. Cite graph manifest IDs when using them. diff --git a/src/application/ArtifactInventory/reader.ts b/src/application/ArtifactInventory/reader.ts index 1e0889c31f..306dbb2b71 100644 --- a/src/application/ArtifactInventory/reader.ts +++ b/src/application/ArtifactInventory/reader.ts @@ -11,6 +11,8 @@ export const createReader = async ( path: string, format: ArtifactNode["format"], signal?: AbortSignal, + /** The caller verifies declared member checksums itself (inventory scans do). */ + options: { readonly callerVerifiesChecksums?: boolean } = {}, ): Promise => { switch (format) { case "directory": @@ -24,7 +26,9 @@ export const createReader = async ( case "asar": return new AsarArtifactReader(path); case "pkg": - return new XarArtifactReader(path); + return new XarArtifactReader(path, { + verifyChecksums: options.callerVerifiesChecksums !== true, + }); case "mach-o-universal": return process.platform === "darwin" ? new MachOSliceArtifactReader(path) diff --git a/src/application/ArtifactInventory/scanCanonical.ts b/src/application/ArtifactInventory/scanCanonical.ts index b02005394b..e668984995 100644 --- a/src/application/ArtifactInventory/scanCanonical.ts +++ b/src/application/ArtifactInventory/scanCanonical.ts @@ -45,7 +45,10 @@ export const scanCanonicalArtifactInventory = async ( const rootDigest = metadata.isDirectory() ? null : await hashReadable(createReadStream(path), options.signal); - const reader = await createReader(path, rootFormat, options.signal); + // The scan applies the integrity policy to declared member checksums. + const reader = await createReader(path, rootFormat, options.signal, { + callerVerifiesChecksums: true, + }); try { const { nodes, occurrences, pendingContradictions } = await scanReader( @@ -148,7 +151,14 @@ const buildInventorySnapshot = async ( edges: orderedEdges, provenance: reader?.provenance() ?? [], integrity_contradictions: integrityContradictions, - limitations: buildLimitations(rootFormat, reader, integrityContradictions), + limitations: buildLimitations( + rootFormat, + reader, + integrityContradictions, + input.occurrences.filter( + ({ hash_status: status }) => status === "mismatched", + ).length, + ), }; }; @@ -261,11 +271,22 @@ const buildLimitations = ( rootFormat: ArtifactNode["format"], reader: ArtifactReader | undefined, integrityContradictions: readonly IntegrityContradiction[], -): string[] => [ - ...inventoryLimitations(rootFormat, reader), - ...(integrityContradictions.length === 0 - ? [] - : [ - `${String(integrityContradictions.length)} integrity contradiction(s) were recorded; mismatched content is observed-untrusted.`, - ]), -]; + mismatchedOccurrences: number, +): string[] => { + // integrity_contradictions carry SHA-256 values; other declared checksums + // (xar SHA-1/MD5) are recorded on their occurrences. + const checksumOnly = mismatchedOccurrences - integrityContradictions.length; + return [ + ...inventoryLimitations(rootFormat, reader), + ...(integrityContradictions.length === 0 + ? [] + : [ + `${String(integrityContradictions.length)} integrity contradiction(s) were recorded; mismatched content is observed-untrusted.`, + ]), + ...(checksumOnly <= 0 + ? [] + : [ + `${String(checksumOnly)} member(s) disagree with a declared non-SHA-256 checksum; their occurrences are mismatched and name the declared and observed values.`, + ]), + ]; +}; diff --git a/src/application/ArtifactInventory/scanReader.ts b/src/application/ArtifactInventory/scanReader.ts index 768e67e4d9..a4c2f7a0c3 100644 --- a/src/application/ArtifactInventory/scanReader.ts +++ b/src/application/ArtifactInventory/scanReader.ts @@ -129,9 +129,7 @@ const visitArtifactEntries = async ( entry.nestedArchive === "gzip-cpio"; context.registry.add(logicalPath, expandable ? "directory" : entry.kind); const occurrence = createOccurrence(entry, logicalPath, null); - let digested: - | { readonly node: ArtifactNode; readonly mismatched: boolean } - | undefined; + let digested: DigestedEntry | undefined; try { digested = await digestArtifactEntry( context, @@ -153,6 +151,7 @@ const visitArtifactEntries = async ( if (digested.mismatched) occurrence.limitations.push( "Declared integrity metadata contradicts observed bytes.", + ...digested.mismatchDetails, ); } context.occurrences.push(occurrence); @@ -185,39 +184,51 @@ const visitArtifactEntries = async ( } }; +interface DigestedEntry { + readonly node: ArtifactNode; + readonly mismatched: boolean; + /** Declared and observed values of a non-SHA-256 checksum that disagreed. */ + readonly mismatchDetails: readonly string[]; +} + const digestArtifactEntry = async ( context: ScanContext, currentReader: ArtifactReader, entry: ArtifactEntry, logicalPath: string, -): Promise< - { readonly node: ArtifactNode; readonly mismatched: boolean } | undefined -> => { +): Promise => { if ( (entry.kind !== "file" && entry.kind !== "slice") || entry.encrypted || entry.contentUnavailable === true ) return undefined; + const checksum = entry.declaredChecksum; const digest = await hashReadable( await currentReader.open(entry, context.signal), context.signal, + checksum?.algorithm, ); - const mismatched = + const sha256Mismatch = entry.declaredSha256 !== null && entry.declaredSha256 !== digest.sha256; - if (mismatched && entry.declaredSha256 !== null) { - if (context.integrity.mode === "fail") - throw new ArtifactReaderFailure( - "integrity", - `Artifact integrity metadata disagrees with content: ${logicalPath}`, - undefined, - { - logicalPath, - declaredSha256: entry.declaredSha256, - calculatedSha256: digest.sha256, - unpacked: entry.unpacked, - }, - ); + const checksumMismatch = + checksum !== undefined && checksum.value !== digest.also; + const mismatched = sha256Mismatch || checksumMismatch; + if (mismatched && context.integrity.mode === "fail") + throw new ArtifactReaderFailure( + "integrity", + sha256Mismatch || checksum === undefined + ? `Artifact integrity metadata disagrees with content: ${logicalPath}` + : `Declared ${checksum.algorithm} checksum disagrees with content: ${logicalPath}`, + undefined, + { + logicalPath, + declaredSha256: entry.declaredSha256, + calculatedSha256: digest.sha256, + unpacked: entry.unpacked, + }, + ); + if (sha256Mismatch && entry.declaredSha256 !== null) context.pendingContradictions.push({ logicalPath, declaredSha256: entry.declaredSha256, @@ -225,7 +236,6 @@ const digestArtifactEntry = async ( entryKind: entry.kind, unpacked: entry.unpacked, }); - } const classified = entry.kind === "slice" ? ({ kind: "universal-slice", format: "mach-o" } as const) @@ -245,6 +255,11 @@ const digestArtifactEntry = async ( : [], }), mismatched, + mismatchDetails: checksumMismatch + ? [ + `Declared ${checksum.algorithm} ${checksum.value} disagrees with observed ${digest.also ?? "unavailable"}.`, + ] + : [], }; }; diff --git a/src/artifacts/ArtifactHash.ts b/src/artifacts/ArtifactHash.ts index b3bf43bfab..dd5cb93d1b 100644 --- a/src/artifacts/ArtifactHash.ts +++ b/src/artifacts/ArtifactHash.ts @@ -9,6 +9,8 @@ export type HashResult = { readonly sha256: string; readonly bytes: number; readonly prefix: Buffer; + /** Digest in the additionally requested algorithm, when one was requested. */ + readonly also?: string; }; /** Bounded classification evidence; this is not an executable format size limit. */ @@ -27,8 +29,11 @@ export const abortIfNeeded = (signal?: AbortSignal): void => { export const hashReadable = async ( stream: Readable, signal?: AbortSignal, + alsoAlgorithm?: string, ): Promise => { const hash = createHash("sha256"); + const also = + alsoAlgorithm === undefined ? undefined : createHash(alsoAlgorithm); const prefixes: Buffer[] = []; let prefixBytes = 0; let bytes = 0; @@ -37,6 +42,7 @@ export const hashReadable = async ( const chunk = streamChunkToBuffer(raw); bytes += chunk.length; hash.update(chunk); + also?.update(chunk); if (prefixBytes < ARTIFACT_CLASSIFICATION_PREFIX_BYTES) { const selected = chunk.subarray( 0, @@ -46,5 +52,10 @@ export const hashReadable = async ( prefixBytes += selected.length; } } - return { sha256: hash.digest("hex"), bytes, prefix: Buffer.concat(prefixes) }; + return { + sha256: hash.digest("hex"), + bytes, + prefix: Buffer.concat(prefixes), + ...(also === undefined ? {} : { also: also.digest("hex") }), + }; }; diff --git a/src/artifacts/ArtifactReader.ts b/src/artifacts/ArtifactReader.ts index c4becbdc80..3f0faeaf3d 100644 --- a/src/artifacts/ArtifactReader.ts +++ b/src/artifacts/ArtifactReader.ts @@ -15,6 +15,15 @@ export interface ArtifactEntry { readonly unpacked: boolean; readonly limitations: readonly string[]; readonly adapterKey: string; + /** + * Container checksum of the member's bytes in an algorithm other than + * SHA-256 (xar declares SHA-1 or MD5). The scan verifies it under the + * caller's integrity policy. + */ + readonly declaredChecksum?: { + readonly algorithm: "sha1" | "md5" | "sha512"; + readonly value: string; + }; /** The container records this member but does not hold its bytes, such as an unresolved hard link. */ readonly contentUnavailable?: boolean; /** Archive format of this member's own bytes, when the scan should expand it. */ diff --git a/src/artifacts/CpioArtifactReader.ts b/src/artifacts/CpioArtifactReader.ts index 565c1fb5a7..43cc446193 100644 --- a/src/artifacts/CpioArtifactReader.ts +++ b/src/artifacts/CpioArtifactReader.ts @@ -13,6 +13,8 @@ const MAX_NAME_BYTES = 64 * 1024; /** Hard-link bytes are buffered to serve every linked path, within these bounds. */ const MAX_LINK_BUFFER_BYTES = 16 * 1024 * 1024; const MAX_LINK_BUFFER_TOTAL = 64 * 1024 * 1024; +/** Longest symlink target read; PATH_MAX is 1024 on macOS and 4096 on Linux. */ +const MAX_SYMLINK_TARGET_BYTES = 4096; const TRAILER = "TRAILER!!!"; const S_IFMT = 0o170000; const S_IFDIR = 0o040000; @@ -278,14 +280,22 @@ export class CpioArtifactReader implements ArtifactReader { const type = header.mode & S_IFMT; const path = memberPath(raw); if (type === S_IFLNK) { - const target = await this.#collect(source, header, raw); + // A hostile header can declare a multi-gigabyte target; stream it past. + let target: Buffer | undefined; + if (header.fileSize > MAX_SYMLINK_TARGET_BYTES) + await drain(this.#verified(source, header, raw)); + else target = await this.#collect(source, header, raw); if (path !== undefined) yield entryOf({ path, kind: "symlink", key, header, - limitations: [`Symlink target: ${target.toString("utf8")}`], + limitations: [ + target === undefined + ? `Symlink target of ${header.fileSize} bytes exceeds ${MAX_SYMLINK_TARGET_BYTES} bytes and was not read.` + : `Symlink target: ${target.toString("utf8")}`, + ], }); return; } diff --git a/src/artifacts/InstallerPackageReaders.test.ts b/src/artifacts/InstallerPackageReaders.test.ts index 4b6c818fbc..11823f1746 100644 --- a/src/artifacts/InstallerPackageReaders.test.ts +++ b/src/artifacts/InstallerPackageReaders.test.ts @@ -272,6 +272,39 @@ describe("installer package reader hardening", () => { }); }); +describe("cpio symlink targets", () => { + it("streams past an oversized symlink target and keeps reading", async () => { + const reader = new CpioArtifactReader(() => + Promise.resolve( + Readable.from([ + gzipCpio( + [ + { name: "./huge", mode: MODE.symlink, data: "x".repeat(4097) }, + { name: "./after", mode: MODE.file, data: "after" }, + ], + "crc", + ), + ]), + ), + ); + try { + const entries = await collect(reader); + expect( + entries.map(({ kind, path, limitations }) => [kind, path, limitations]), + ).toEqual([ + [ + "symlink", + "huge", + ["Symlink target of 4097 bytes exceeds 4096 bytes and was not read."], + ], + ["file", "after", []], + ]); + } finally { + await reader.close(); + } + }); +}); + describe("cpio hard links and CRC archives", () => { const readAll = async (archive: Uint8Array) => { const reader = new CpioArtifactReader(() => diff --git a/src/artifacts/XarArtifactReader.ts b/src/artifacts/XarArtifactReader.ts index f11798dd66..b6f858932e 100644 --- a/src/artifacts/XarArtifactReader.ts +++ b/src/artifacts/XarArtifactReader.ts @@ -151,6 +151,22 @@ const collectMembers = (toc: Element): XarMember[] => { return members; }; +/** Expose a member's extracted checksum so a caller can apply its own policy. */ +const declaredDigest = ( + checksum: { readonly algorithm: string; readonly value: string } | undefined, +): Pick => { + if (checksum === undefined) return { declaredSha256: null }; + if (checksum.algorithm === "sha256") + return { declaredSha256: checksum.value }; + const algorithm = checksum.algorithm; + return algorithm === "sha1" || algorithm === "md5" || algorithm === "sha512" + ? { + declaredSha256: null, + declaredChecksum: { algorithm, value: checksum.value }, + } + : { declaredSha256: null }; +}; + /** Verify a member's extracted checksum as its decoded bytes stream past. */ class ChecksumVerifier extends Transform { readonly #hash: Hash; @@ -203,7 +219,19 @@ export class XarArtifactReader implements ArtifactReader { #heap = 0; #members: readonly XarMember[] = []; - constructor(private readonly path: string) {} + readonly #verifyChecksums: boolean; + + /** + * @param options.verifyChecksums Fail `open()` streams whose bytes disagree + * with the member's extracted checksum (default). Inventory passes false and + * verifies the declared checksum itself under the caller's integrity policy. + */ + constructor( + private readonly path: string, + options: { readonly verifyChecksums?: boolean } = {}, + ) { + this.#verifyChecksums = options.verifyChecksums ?? true; + } async #load(signal?: AbortSignal): Promise { if (this.#handle !== undefined) return; @@ -364,8 +392,7 @@ export class XarArtifactReader implements ArtifactReader { member.data?.encoding === "application/octet-stream" ? this.#heap + member.data.offset : null, - // xar declares SHA-1/MD5 checksums, which open() verifies itself. - declaredSha256: null, + ...declaredDigest(member.data?.extractedChecksum), unpacked: false, limitations: [ ...(member.kind === "symlink" && member.link !== undefined @@ -409,7 +436,7 @@ export class XarArtifactReader implements ArtifactReader { ); const checksum = data.extractedChecksum; const verified = - checksum === undefined + checksum === undefined || !this.#verifyChecksums ? new PassThrough() : new ChecksumVerifier(entry.path, checksum); // Decoder errors become tagged failures; cancellation stays tagged. diff --git a/tests/boundary/filesystem/installerPackageInventory.test.ts b/tests/boundary/filesystem/installerPackageInventory.test.ts index 8940d87e77..7d1a2f46ed 100644 --- a/tests/boundary/filesystem/installerPackageInventory.test.ts +++ b/tests/boundary/filesystem/installerPackageInventory.test.ts @@ -114,10 +114,46 @@ describe("installer package inventory", () => { await runProviderAnalysis(path, "inventory_artifact", {}), ).toMatchObject({ error: "Analysis failed", - details: { reason: "integrity" }, + code: "artifact_integrity_mismatch", + details: { logical_path: "App.pkg/PackageInfo", declared_sha256: null }, }); }); + it("records a xar checksum mismatch under record-and-continue", async () => { + const directory = await createTestTempDirectory("rea-pkg-recorded-"); + const path = join(directory, "Installer.pkg"); + await writeFile(path, productPackage("", "0".repeat(40))); + const inventory = artifactInventoryResultSchema.parse( + parseEvidence( + await runProviderAnalysis(path, "inventory_artifact", { + integrity_policy: "record-and-continue", + }), + ).normalized_result, + ); + const info = inventory.occurrences.find( + ({ logical_path: logical }) => logical === "App.pkg/PackageInfo", + ); + const observedSha1 = createHash("sha1") + .update('') + .digest("hex"); + expect(info).toMatchObject({ hash_status: "mismatched" }); + expect(info?.limitations).toContain( + `Declared sha1 ${"0".repeat(40)} disagrees with observed ${observedSha1}.`, + ); + expect(inventory.limitations).toContainEqual( + expect.stringContaining( + "1 member(s) disagree with a declared non-SHA-256 checksum", + ), + ); + // Untampered members are still inventoried and expanded. + expect( + inventory.occurrences.some( + ({ logical_path: logical }) => + logical === "App.pkg/Scripts/postinstall", + ), + ).toBe(true); + }); + it("records an unresolved hard link as unavailable instead of empty", async () => { const directory = await createTestTempDirectory("rea-pkg-hardlink-"); const path = join(directory, "Installer.pkg");