From a2235b09acce184570f5287d650906e3981f0baa Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 04:59:12 +0800 Subject: [PATCH] fix(apk): classify damaged Zip64 tail framing Separate malformed locators and fixed records from unsupported disk layouts or numeric representation limits at the ZIP tail owner. Preserve the classic 65535-entry fallback and existing downstream parsed facts. Add public CLI and stdio MCP valid, damaged, restored and unsupported controls; document the bounded validation scope. Fixes #6471 --- docs/apk-signature.md | 19 +- src/domain/apkSignature/apkSigningBlock.ts | 46 ++++- tests/acceptance/apkZip64FramingCli.test.ts | 193 ++++++++++++++++++++ tests/fixtures/apkSignature/zip64Tail.ts | 46 +++++ 4 files changed, 291 insertions(+), 13 deletions(-) create mode 100644 tests/acceptance/apkZip64FramingCli.test.ts create mode 100644 tests/fixtures/apkSignature/zip64Tail.ts diff --git a/docs/apk-signature.md b/docs/apk-signature.md index b25482022e..b2f04c4f9c 100644 --- a/docs/apk-signature.md +++ b/docs/apk-signature.md @@ -214,9 +214,9 @@ Not yet read: v4 `.apk.idsig` (a sibling file), verity (`0x0421`/`0x0423`/`0x042 ## Bounds and errors The archive is read once, bounded at 512 MiB, without following symlinks and -with change detection. Every size and offset in the ZIP tail and signing block -is checked against its containing section before use, including the complete -central-directory extent before ZIP end records. A process-wide queue permits +with change detection. ZIP tail offsets, the central-directory size and signing +block spans used by the reader are checked against their containing sections, +including the complete central-directory extent before ZIP end records. A process-wide queue permits one active whole-file inspection and eight pending scans across provider instances; pending scans retain no APK bytes and can be cancelled with their original reason. Further requests receive a capacity error. The active slot @@ -243,8 +243,17 @@ A file that is not a ZIP is refused as an unsupported target. A damaged ZIP (local headers but no end-of-central-directory), a central directory that cannot be walked or whose declared record count does not account for all of its bytes, or a malformed signing block or signer is an input error naming the -scheme and reason. Signer framing includes signature records, the public-key -field and matching signed/outer v3 SDK ranges. Suffix bytes beyond known +offset or scheme and reason. Zip64 tail damage includes a missing required +locator, an impossible locator disk/count pair, a fixed end record that does +not fit before its locator, or an incorrect end-record signature. These are +input errors, not evidence of an unsupported format. Coherent multi-disk +locator metadata and 64-bit offsets/counts/sizes beyond the reader's safe +integer range remain unsupported. The locator offset is disk-relative, so +multi-disk metadata is checked before applying local file bounds. This follows +the record fields in [PKWARE APPNOTE 6.3.10, sections 4.3.14–4.3.15](https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT); +it does not establish full validation of Zip64 versions or extensible data. +Signer framing includes signature records, the public-key field and matching +signed/outer v3 SDK ranges. Suffix bytes beyond known fields are counted as uninterpreted, as Android readers may ignore extensions; their presence does not establish malformed signing. When the central directory is not immediately followed by diff --git a/src/domain/apkSignature/apkSigningBlock.ts b/src/domain/apkSignature/apkSigningBlock.ts index 58b6706955..72869abc19 100644 --- a/src/domain/apkSignature/apkSigningBlock.ts +++ b/src/domain/apkSignature/apkSigningBlock.ts @@ -142,19 +142,49 @@ const centralDirectoryOffset = ( if (offset !== ZIP64_SENTINEL && size !== ZIP64_SENTINEL) return ok({ offset, size, entries, end: eocdOffset }); return err({ - kind: "zip64-unsupported", + kind: "malformed-zip", + offset: eocdOffset, detail: "missing Zip64 EOCD locator", }); } - const recordOffset = u64(dv, locator + 8); - if ( - recordOffset === null || - recordOffset + 56 > locator || - dv.getUint32(recordOffset, true) !== ZIP64_EOCD_SIGNATURE - ) + // APPNOTE 6.3.10 §4.3.15 makes the record offset relative to its disk. + // A selected file cannot resolve another disk's offset as a local span. + const recordDisk = dv.getUint32(locator + 4, true); + const totalDisks = dv.getUint32(locator + 16, true); + if (totalDisks === 0) + return err({ + kind: "malformed-zip", + offset: locator + 16, + detail: "Zip64 locator declares zero disks", + }); + if (recordDisk >= totalDisks) + return err({ + kind: "malformed-zip", + offset: locator + 4, + detail: "Zip64 EOCD disk is outside the declared disk count", + }); + if (totalDisks > 1) return err({ kind: "zip64-unsupported", - detail: "malformed Zip64 EOCD record", + detail: "multi-disk Zip64 archives are not supported", + }); + const recordOffset = u64(dv, locator + 8); + if (recordOffset === null) + return err({ + kind: "zip64-unsupported", + detail: "Zip64 EOCD offset exceeds safe range", + }); + if (recordOffset + 56 > locator) + return err({ + kind: "malformed-zip", + offset: locator + 8, + detail: "Zip64 EOCD record extends beyond its locator", + }); + if (dv.getUint32(recordOffset, true) !== ZIP64_EOCD_SIGNATURE) + return err({ + kind: "malformed-zip", + offset: recordOffset, + detail: "invalid Zip64 EOCD signature", }); const z64Offset = u64(dv, recordOffset + 48); const z64Size = u64(dv, recordOffset + 40); diff --git a/tests/acceptance/apkZip64FramingCli.test.ts b/tests/acceptance/apkZip64FramingCli.test.ts new file mode 100644 index 0000000000..ea02f724a7 --- /dev/null +++ b/tests/acceptance/apkZip64FramingCli.test.ts @@ -0,0 +1,193 @@ +import { createHash } from "node:crypto"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; + +import { analysisErrorProjectionSchema } from "../../src/contracts/errorSchemas.js"; +import { apkSignatureResultSchema } from "../../src/domain/apkSignature/apkSignatureReport.js"; +import { parseEvidence } from "../../src/domain/evidence.js"; +import { zip64Tail } from "../fixtures/apkSignature/zip64Tail.js"; +import { parseMcpToolError } from "../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; +import { cliTest, type TestCli } from "../support/cli/cliFixture.js"; + +const inspectBoth = async (cli: TestCli, path: string) => { + const output = await cli.sharedCliRun!({ + arguments: [ + "inspect-apk-signature", + path, + "--detail", + "complete", + "--json", + ], + }); + const call = await cli.sharedMcpCall!(); + const response = await call("inspect_apk_signature", { + path, + detail: "complete", + }); + return { output, response }; +}; + +const expectUnsignedZip64 = async ( + cli: TestCli, + path: string, + bytes: Uint8Array, +) => { + await writeFile(path, bytes); + const { output, response } = await inspectBoth(cli, path); + expect(output.exitCode, output.stderr).toBe(0); + expect(response.isError, JSON.stringify(response)).not.toBe(true); + const sha256 = createHash("sha256").update(bytes).digest("hex"); + for (const value of [output.json, response.structuredContent]) { + const evidence = parseEvidence(value); + expect(evidence.subject).toMatchObject({ + local_path: path, + digest: { sha256 }, + }); + expect( + apkSignatureResultSchema.parse(evidence.normalized_result), + ).toMatchObject({ + artifact: { path, bytes: bytes.length, sha256 }, + schemes: { + v1: { present: false }, + v2: { present: false }, + v3: { present: false }, + "v3.1": { present: false }, + }, + runtime_execution: "not-performed", + }); + } + expect(await readFile(path)).toEqual(Buffer.from(bytes)); +}; + +const expectRefusal = async ( + cli: TestCli, + path: string, + bytes: Uint8Array, + expected: Record, +) => { + await writeFile(path, bytes); + const { output, response } = await inspectBoth(cli, path); + expect(output.exitCode, output.stderr).toBe(1); + const cliError = analysisErrorProjectionSchema.parse(output.json); + expect(cliError).toMatchObject(expected); + const mcpError = parseMcpToolError(response).error; + expect(mcpError).toMatchObject(expected); + expect(mcpError.message).toBe(cliError.message); + expect(await readFile(path)).toEqual(Buffer.from(bytes)); +}; + +cliTest( + "classifies damaged Zip64 framing as input errors through CLI and stdio MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-apk-zip64-framing-"); + const path = join(root, "selected.apk"); + const fixture = await zip64Tail(); + const { recordOffset, locatorOffset, eocdOffset } = fixture; + await expectUnsignedZip64(cli, path, fixture.bytes); + const cases = [ + { + at: locatorOffset, + value: 0x07064b51, + diagnosticOffset: eocdOffset, + detail: "missing Zip64 EOCD locator", + }, + { + at: recordOffset, + value: 0x06064b51, + diagnosticOffset: recordOffset, + detail: "invalid Zip64 EOCD signature", + }, + { + at: locatorOffset + 8, + value: recordOffset + 1, + diagnosticOffset: locatorOffset + 8, + detail: "Zip64 EOCD record extends beyond its locator", + }, + { + at: locatorOffset + 16, + value: 0, + diagnosticOffset: locatorOffset + 16, + detail: "Zip64 locator declares zero disks", + }, + { + at: locatorOffset + 4, + value: 1, + diagnosticOffset: locatorOffset + 4, + detail: "Zip64 EOCD disk is outside the declared disk count", + }, + ]; + for (const scenario of cases) { + const bytes = Uint8Array.from(fixture.bytes); + new DataView(bytes.buffer).setUint32(scenario.at, scenario.value, true); + await expectRefusal(cli, path, bytes, { + code: "invalid_request", + category: "invalid_input", + retryable: false, + recovery: { + kind: "inspect_input_contract", + operation: "inspect_apk_signature", + }, + details: { + issues: [ + { + path: ["path"], + reason: "invalid_value", + message: `Damaged ZIP/APK at byte ${String(scenario.diagnosticOffset)}: ${scenario.detail}.`, + }, + ], + }, + }); + } + // Restore the exact original bytes at the same caller-selected path. + await expectUnsignedZip64(cli, path, fixture.bytes); + }, +); + +cliTest( + "keeps Zip64 representation limits and non-ZIP targets distinct from damaged input", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-apk-zip64-unsupported-"); + const path = join(root, "selected.apk"); + const fixture = await zip64Tail(); + for (const at of [ + fixture.locatorOffset + 8, + fixture.recordOffset + 32, + fixture.recordOffset + 40, + fixture.recordOffset + 48, + ]) { + const bytes = Uint8Array.from(fixture.bytes); + new DataView(bytes.buffer).setBigUint64(at, 1n << 53n, true); + await expectRefusal(cli, path, bytes, { + code: "unsupported_target", + category: "unsupported_target", + message: expect.stringContaining("safe range"), + details: { path }, + }); + } + const split = Uint8Array.from(fixture.bytes); + const view = new DataView(split.buffer); + view.setUint32(fixture.locatorOffset + 4, 1, true); + view.setUint32(fixture.locatorOffset + 16, 2, true); + view.setBigUint64(fixture.locatorOffset + 8, 1000000n, true); + // Only locator metadata is authored here; this is not a real split-archive oracle. + await expectRefusal(cli, path, split, { + code: "unsupported_target", + category: "unsupported_target", + message: expect.stringContaining("multi-disk Zip64"), + details: { path }, + }); + await expectRefusal( + cli, + path, + new TextEncoder().encode("not a ZIP archive at all"), + { + code: "unsupported_target", + category: "unsupported_target", + message: expect.stringContaining("not a ZIP/APK"), + details: { path }, + }, + ); + }, +); diff --git a/tests/fixtures/apkSignature/zip64Tail.ts b/tests/fixtures/apkSignature/zip64Tail.ts new file mode 100644 index 0000000000..c932b65cff --- /dev/null +++ b/tests/fixtures/apkSignature/zip64Tail.ts @@ -0,0 +1,46 @@ +import { zipWith } from "./v1SignatureBlock.js"; + +/** A real stored entry with an authored APPNOTE 6.3.10 §4.3.14–15 Zip64 tail. */ +export const zip64Tail = async () => { + const classic = await zipWith( + [{ name: "entry.txt", data: "inert", stored: true }], + { dataDescriptor: false }, + ); + const original = new DataView( + classic.buffer, + classic.byteOffset, + classic.byteLength, + ); + const recordOffset = classic.length - 22; + const locatorOffset = recordOffset + 56; + const eocdOffset = locatorOffset + 20; + const bytes = new Uint8Array(classic.length + 76); + bytes.set(classic.subarray(0, recordOffset)); + bytes.set(classic.subarray(recordOffset), eocdOffset); + const view = new DataView(bytes.buffer); + view.setUint32(recordOffset, 0x06064b50, true); + view.setBigUint64(recordOffset + 4, 44n, true); + view.setUint16(recordOffset + 12, 45, true); + view.setUint16(recordOffset + 14, 45, true); + const entries = BigInt(original.getUint16(recordOffset + 10, true)); + view.setBigUint64(recordOffset + 24, entries, true); + view.setBigUint64(recordOffset + 32, entries, true); + view.setBigUint64( + recordOffset + 40, + BigInt(original.getUint32(recordOffset + 12, true)), + true, + ); + view.setBigUint64( + recordOffset + 48, + BigInt(original.getUint32(recordOffset + 16, true)), + true, + ); + view.setUint32(locatorOffset, 0x07064b50, true); + view.setBigUint64(locatorOffset + 8, BigInt(recordOffset), true); + view.setUint32(locatorOffset + 16, 1, true); + view.setUint16(eocdOffset + 8, 0xffff, true); + view.setUint16(eocdOffset + 10, 0xffff, true); + view.setUint32(eocdOffset + 12, 0xffffffff, true); + view.setUint32(eocdOffset + 16, 0xffffffff, true); + return { bytes, recordOffset, locatorOffset, eocdOffset }; +};