diff --git a/src/artifacts/CpioArtifactReader.ts b/src/artifacts/CpioArtifactReader.ts index 43cc446193..6ed472ac8b 100644 --- a/src/artifacts/CpioArtifactReader.ts +++ b/src/artifacts/CpioArtifactReader.ts @@ -233,9 +233,24 @@ export class CpioArtifactReader implements ArtifactReader { ) {} async *entries(signal?: AbortSignal): AsyncIterable { - const source = new ByteSource( - (await this.openCompressed(signal)).pipe(createGunzip()), - ); + const compressed = await this.openCompressed(signal); + const gunzip = createGunzip(); + // `pipe()` does not forward source errors (cancellation, truncation); + // propagate them so the MCP process fails as a tagged Artifact failure. + compressed.on("error", (cause: unknown) => { + gunzip.destroy( + cause instanceof ArtifactReaderFailure + ? cause + : new ArtifactReaderFailure("format", "cpio stream failed", { + cause, + }), + ); + }); + gunzip.on("error", () => { + // Prevent unhandled source errors after the decoder fails first. + compressed.destroy(); + }); + const source = new ByteSource(compressed.pipe(gunzip)); this.#source = source; for (let index = 0; ; index++) { if (signal?.aborted === true) @@ -257,12 +272,18 @@ export class CpioArtifactReader implements ArtifactReader { "name padding", ), ); - const end = nameBytes.indexOf(0); - const raw = nameBytes.toString( - "utf8", - 0, - end >= 0 ? end : nameBytes.length, - ); + // cpio names require exactly one terminal NUL: a missing terminator or + // an embedded NUL with trailing bytes is malformed. + if ( + nameBytes.length === 0 || + nameBytes[nameBytes.length - 1] !== 0 || + nameBytes.indexOf(0) !== nameBytes.length - 1 + ) + throw new ArtifactReaderFailure( + "format", + "cpio member name is not NUL-terminated", + ); + const raw = nameBytes.toString("utf8", 0, nameBytes.length - 1); if (raw === TRAILER) { yield* this.#unresolvedLinks(); return; @@ -305,12 +326,19 @@ export class CpioArtifactReader implements ArtifactReader { header.links > 1 && header.fileSize === 0 ) { + // Zero-size headers carry no bytes, so a CRC archive must declare zero. + if (header.format === "crc" && header.check !== 0) + throw new ArtifactReaderFailure( + "integrity", + `cpio CRC disagrees with content: ${raw}`, + ); const stored = this.#links.get(header.identity); if (stored !== undefined) { yield this.#alias(path, key, header, stored); return; } // newc stores a hard link's bytes on its last member; wait for them. + // An all-empty group is resolved to empty files at TRAILER!!!. const waiting = this.#pending.get(header.identity) ?? []; waiting.push({ path, key, header }); this.#pending.set(header.identity, waiting); @@ -318,7 +346,26 @@ export class CpioArtifactReader implements ArtifactReader { } this.#current = { key, path: raw, header }; this.#consumed = false; - if (path === undefined || (type !== S_IFREG && type !== S_IFDIR)) return; + if (path === undefined) return; + if (type !== S_IFREG && type !== S_IFDIR) { + // FIFOs, device nodes, sockets and other types are not expanded; keep + // an explicit unavailable occurrence instead of dropping the path. + yield entryOf({ + path, + kind: "file", + key, + header, + size: null, + limitations: [ + `Unsupported cpio member type ${type.toString(8)}; content not expanded.`, + ], + contentUnavailable: true, + }); + // Consume its data (if any) so the next header aligns. + await drain(this.#verified(source, header, raw)); + this.#current = undefined; + return; + } yield entryOf({ path, kind: type === S_IFDIR ? "directory" : "file", @@ -349,7 +396,27 @@ export class CpioArtifactReader implements ArtifactReader { *#unresolvedLinks(): Generator { for (const links of this.#pending.values()) - for (const { path, key, header } of links) + for (const { path, key, header } of links) { + // A complete zero-length group establishes empty content. An orphan + // that claims more links than appear is missing its bytes. + const complete = + links.length >= header.links && + links.every((m) => m.header.fileSize === 0); + if (header.fileSize === 0 && complete) { + const empty = Buffer.alloc(0); + this.#aliases.set(key, empty); + yield entryOf({ + path, + kind: "file", + key, + header, + size: 0, + limitations: [ + "Hard link: every member of this link group is empty.", + ], + }); + continue; + } yield entryOf({ path, kind: "file", @@ -359,6 +426,7 @@ export class CpioArtifactReader implements ArtifactReader { limitations: [UNRESOLVED_LINK], contentUnavailable: true, }); + } this.#pending.clear(); } diff --git a/src/artifacts/InstallerPackageReaders.test.ts b/src/artifacts/InstallerPackageReaders.test.ts index 11823f1746..22b6473740 100644 --- a/src/artifacts/InstallerPackageReaders.test.ts +++ b/src/artifacts/InstallerPackageReaders.test.ts @@ -199,6 +199,7 @@ describe("gzip cpio reader", () => { "directory:usr", "file:usr/bin/tool", "symlink:usr/lib/link", + "file:usr/fifo", "file:usr/share/readme.txt", ]); }); diff --git a/src/artifacts/XarArtifactReader.ts b/src/artifacts/XarArtifactReader.ts index b6f858932e..64d7016b7c 100644 --- a/src/artifacts/XarArtifactReader.ts +++ b/src/artifacts/XarArtifactReader.ts @@ -49,11 +49,19 @@ interface XarData { readonly extractedChecksum: | { readonly algorithm: string; readonly value: string } | undefined; + readonly unsupportedChecksum: + | { readonly style: string; readonly value: string } + | undefined; + readonly archivedChecksum: + | { readonly algorithm: string; readonly value: string } + | undefined; } interface XarMember { readonly path: string; readonly kind: "file" | "directory" | "symlink"; + /** Raw TOC type when it is not file/directory/symlink (FIFO, device, ...). */ + readonly unsupportedType?: string; readonly mode: number | null; readonly data: XarData | undefined; readonly link: string | undefined; @@ -83,11 +91,27 @@ const integer = (value: string | undefined, label: string): number => { return parsed; }; +const parseChecksum = ( + parent: Element, + tag: string, +): + | { readonly algorithm: string; readonly value: string } + | { readonly unsupported: string; readonly value: string } + | undefined => { + const element = childElement(parent, tag); + if (element === undefined) return undefined; + const style = element.getAttribute("style")?.toLowerCase() ?? ""; + const value = (element.textContent ?? "").trim().toLowerCase(); + if (CHECKSUM_ALGORITHMS[style] === undefined) + return { unsupported: style || "(missing style)", value }; + return { algorithm: style, value }; +}; + const parseData = (file: Element): XarData | undefined => { const data = childElement(file, "data"); if (data === undefined) return undefined; - const checksum = childElement(data, "extracted-checksum"); - const style = checksum?.getAttribute("style")?.toLowerCase() ?? ""; + const extracted = parseChecksum(data, "extracted-checksum"); + const archived = parseChecksum(data, "archived-checksum"); return { offset: integer(textOf(data, "offset"), "data offset"), length: integer(textOf(data, "length"), "data length"), @@ -96,12 +120,17 @@ const parseData = (file: Element): XarData | undefined => { childElement(data, "encoding")?.getAttribute("style") ?? "application/octet-stream", extractedChecksum: - checksum === undefined || CHECKSUM_ALGORITHMS[style] === undefined + extracted === undefined || "unsupported" in extracted ? undefined - : { - algorithm: style, - value: (checksum.textContent ?? "").trim().toLowerCase(), - }, + : { algorithm: extracted.algorithm, value: extracted.value }, + unsupportedChecksum: + extracted !== undefined && "unsupported" in extracted + ? { style: extracted.unsupported, value: extracted.value } + : undefined, + archivedChecksum: + archived === undefined || "unsupported" in archived + ? undefined + : { algorithm: archived.algorithm, value: archived.value }, }; }; @@ -134,12 +163,25 @@ const collectMembers = (toc: Element): XarMember[] => { ? "directory" : type === "symlink" ? "symlink" - : "file"; + : type === undefined || type === "file" + ? "file" + : "file"; + const unsupportedType = + type !== undefined && + type !== "file" && + type !== "directory" && + type !== "symlink" + ? type + : undefined; members.push({ path, kind, + ...(unsupportedType === undefined ? {} : { unsupportedType }), mode: Number.isSafeInteger(mode) ? mode : null, - data: kind === "file" ? parseData(next.element) : undefined, + data: + kind === "file" && unsupportedType === undefined + ? parseData(next.element) + : undefined, link: kind === "symlink" ? textOf(next.element, "link") : undefined, }); pending.push( @@ -167,6 +209,47 @@ const declaredDigest = ( : { declaredSha256: null }; }; +/** Bound decoded output to the TOC-declared extracted size. */ +class SizeBound extends Transform { + #seen = 0; + + constructor( + private readonly path: string, + private readonly expected: number, + ) { + super(); + } + + override _transform( + chunk: Buffer, + _encoding: BufferEncoding, + done: TransformCallback, + ): void { + this.#seen += chunk.length; + if (this.#seen > this.expected) { + done( + new ArtifactReaderFailure( + "limit", + `xar member ${this.path} decodes beyond its declared ${this.expected} bytes`, + ), + ); + return; + } + done(null, chunk); + } + + override _flush(done: TransformCallback): void { + done( + this.#seen === this.expected + ? null + : new ArtifactReaderFailure( + "format", + `xar member ${this.path} decoded ${this.#seen} bytes, expected ${this.expected}`, + ), + ); + } +} + /** Verify a member's extracted checksum as its decoded bytes stream past. */ class ChecksumVerifier extends Transform { readonly #hash: Hash; @@ -295,9 +378,14 @@ export class XarArtifactReader implements ArtifactReader { /** The TOC checksum in the heap covers the compressed TOC bytes. */ async #verifyToc(toc: Element, compressed: Buffer): Promise { const checksum = childElement(toc, "checksum"); + if (checksum === undefined) return; const style = checksum?.getAttribute("style")?.toLowerCase() ?? ""; const algorithm = CHECKSUM_ALGORITHMS[style]; - if (checksum === undefined || algorithm === undefined) return; + if (algorithm === undefined) + throw new ArtifactReaderFailure( + "format", + `xar TOC declares unsupported checksum style ${style || "(missing)"}`, + ); const size = integer(textOf(checksum, "size"), "checksum size"); // The stored checksum is exactly one digest; never allocate a declared size. if (size !== DIGEST_BYTES[style]) @@ -377,10 +465,45 @@ export class XarArtifactReader implements ArtifactReader { await this.#load(signal); for (const member of this.#members) { cancelled(signal); + if (member.unsupportedType !== undefined) { + yield { + path: member.path, + kind: "file", + declaredSize: null, + compressedSize: null, + executable: false, + encrypted: false, + byteOffset: null, + declaredSha256: null, + unpacked: false, + limitations: [ + `Unsupported xar member type ${member.unsupportedType}; content not expanded.`, + ], + adapterKey: member.path, + contentUnavailable: true, + }; + continue; + } + const data = member.data; + // Raw members must describe their stored bytes exactly; otherwise the + // source range would extend into adjacent heap entries. + if ( + data !== undefined && + data.encoding === "application/octet-stream" && + data.size !== data.length + ) + throw new ArtifactReaderFailure( + "format", + `xar member ${member.path} declares size ${data.size} with length ${data.length}`, + ); + const unsupportedChecksum = data?.unsupportedChecksum; const classified = - member.kind === "file" + member.kind === "file" && unsupportedChecksum === undefined ? await this.#nestedArchive(member) - : { nested: undefined, limitations: [] }; + : { + nested: undefined as XarNestedArchive | undefined, + limitations: [] as readonly string[], + }; yield { path: member.path, kind: member.kind, @@ -398,6 +521,11 @@ export class XarArtifactReader implements ArtifactReader { ...(member.kind === "symlink" && member.link !== undefined ? [`Symlink target recorded in the TOC: ${member.link}`] : []), + ...(unsupportedChecksum === undefined + ? [] + : [ + `Member declares unsupported checksum ${unsupportedChecksum.style}; integrity not verified.`, + ]), ...classified.limitations, ], adapterKey: member.path, @@ -417,18 +545,50 @@ export class XarArtifactReader implements ArtifactReader { "path", `xar member is not a file: ${entry.path}`, ); + if (member.unsupportedType !== undefined) + throw new ArtifactReaderFailure( + "unavailable", + `xar member ${entry.path} has unsupported type ${member.unsupportedType}`, + ); const data = member.data; if (data === undefined) return Readable.from([]); + if (entry.contentUnavailable === true) + throw new ArtifactReaderFailure( + "unavailable", + `xar member ${entry.path} is not expandable`, + ); + // Raw members must describe their stored bytes exactly. + if ( + data.encoding === "application/octet-stream" && + data.size !== data.length + ) + throw new ArtifactReaderFailure( + "format", + `xar member ${entry.path} declares size ${data.size} with length ${data.length}`, + ); + const archived = data.archivedChecksum; + const archivedVerifier = + archived === undefined || !this.#verifyChecksums + ? undefined + : new ChecksumVerifier(`${entry.path} (archived)`, archived); const raw = Readable.from(this.#chunks(data.offset, data.length, signal)); + const archivedChecked = + archivedVerifier === undefined ? raw : raw.pipe(archivedVerifier); let decoded: Readable; - if (data.encoding === "application/octet-stream") decoded = raw; + if (data.encoding === "application/octet-stream") decoded = archivedChecked; else if (data.encoding === "application/x-gzip") { - // xar's "x-gzip" members are zlib streams. + // xar's "x-gzip" members are zlib streams; bound decoded output to the + // declared extracted size so a gzip bomb cannot exhaust the process. const inflate = createInflate(); raw.on("error", (cause: unknown) => inflate.destroy(memberFailure(entry.path, cause)), ); - decoded = raw.pipe(inflate); + const inflated = archivedChecked.pipe(inflate); + const bounded = new SizeBound(entry.path, data.size); + inflated.on("error", (cause: unknown) => + bounded.destroy(memberFailure(entry.path, cause)), + ); + decoded = inflated.pipe(bounded); } else throw new ArtifactReaderFailure( "format", diff --git a/src/artifacts/inventory/scanReader.ts b/src/artifacts/inventory/scanReader.ts index 02f15145e2..8206f89f5b 100644 --- a/src/artifacts/inventory/scanReader.ts +++ b/src/artifacts/inventory/scanReader.ts @@ -204,11 +204,42 @@ const digestArtifactEntry = async ( ) return undefined; const checksum = entry.declaredChecksum; - const digest = await hashReadable( - await currentReader.open(entry, context.signal), - context.signal, - checksum?.algorithm, - ); + let digest: Awaited>; + try { + digest = await hashReadable( + await currentReader.open(entry, context.signal), + context.signal, + checksum?.algorithm, + ); + } catch (cause: unknown) { + // A CRC, archived-checksum, or size-bound failure while streaming is an + // integrity mismatch for this member, not the whole inspection, when the + // caller selected record-and-continue. + if ( + context.integrity.mode !== "record-and-continue" || + !(cause instanceof ArtifactReaderFailure) || + (cause.reason !== "integrity" && + cause.reason !== "limit" && + cause.reason !== "format") + ) + throw cause; + return { + node: createArtifactNode({ + sha256: + "0000000000000000000000000000000000000000000000000000000000000000", + size: 0, + kind: "resource", + format: "unknown", + executable: entry.executable, + contentState: "embedded", + limitations: [ + "Observed content contradicts declared integrity metadata and is untrusted.", + ], + }), + mismatched: true, + mismatchDetails: [cause.message], + }; + } const sha256Mismatch = entry.declaredSha256 !== null && entry.declaredSha256 !== digest.sha256; const checksumMismatch =