diff --git a/src/apktool/ApktoolProvider.test.ts b/src/apktool/ApktoolProvider.test.ts index 5b2a32114d..b6ee249e54 100644 --- a/src/apktool/ApktoolProvider.test.ts +++ b/src/apktool/ApktoolProvider.test.ts @@ -96,6 +96,7 @@ const decodeStub = ( readonly withStrings?: boolean; readonly withGerman?: boolean; readonly failDecode?: boolean; + readonly ymlFile?: string; readonly manifestFile?: string; readonly stringsFile?: string; readonly stringsXml?: string; @@ -104,6 +105,10 @@ const decodeStub = ( const withStrings = options.withStrings ?? true; const withGerman = options.withGerman ?? true; const stringsXml = options.stringsXml ?? REAL_STRINGS; + const ymlWrite = + options.ymlFile === undefined + ? `printf '%s\\n' '${REAL_YML.replace(/'/gu, "'\\''")}' > "$5/apktool.yml"` + : `cp '${options.ymlFile}' "$5/apktool.yml"`; const manifestWrite = options.manifestFile === undefined ? `printf '%s\\n' '${REAL_MANIFEST.replace(/'/gu, "'\\''")}' > "$5/AndroidManifest.xml"` @@ -117,7 +122,7 @@ if [ "$1" = "d" ]; then ${ options.failDecode === true ? `printf 'Exception in thread "main" brut.androlib.AndrolibException: Could not decode the APK\\n' >&2; exit 1;` - : `printf '%s\\n' '${REAL_YML.replace(/'/gu, "'\\''")}' > "$5/apktool.yml" + : `${ymlWrite} ${manifestWrite} mkdir -p "$5/res/values"${withGerman ? ' "$5/res/values-de"' : ""} ${withStrings ? stringsWrite : ""} @@ -192,6 +197,47 @@ it("decodes resources with target identity, metadata, and strings", async () => expect(result.coverage).toBe("complete"); }); +it.each([ + { + name: "malformed UTF-8", + bytes: Buffer.concat([ + Buffer.from("versionInfo:\n versionName: com."), + Buffer.from([0xff]), + Buffer.from(".app\n"), + ]), + valid: false, + }, + { + name: "a literal replacement character", + bytes: Buffer.from("versionInfo:\n versionName: com.�.app\n", "utf8"), + valid: true, + }, +])("handles $name in producer metadata", async ({ bytes, valid }) => { + const { command, root } = await writeApktoolStub((root) => + decodeStub({ ymlFile: join(root, "malformed-apktool.yml") })(root), + ); + await writeFile(join(root, "malformed-apktool.yml"), bytes); + const apk = await apkFixture(root); + if (valid) { + const result = await evidenceResult(providerOver(command), { + operation: "decode_android_resources", + input: { path: apk.path, include_strings: false }, + }); + expect(result.metadata).toMatchObject({ version_name: "com.�.app" }); + expect(result.coverage).toBe("complete"); + return; + } + const result = await providerOver(command).execute({ + operation: "decode_android_resources", + input: { path: apk.path, include_strings: false }, + }); + if (result.ok) throw new Error("expected invalid producer output"); + const projected = projectAnalysisError(result.error); + expect(projected).toMatchObject({ code: "unreadable_output" }); + expect(result.error.message).toContain("invalid UTF-8"); + expect(result.error.message).toContain("apktool.yml"); +}); + it("preserves empty resource strings and reports markup omissions as partial", async () => { const stringsXml = ' A & B Hello world'; diff --git a/src/apktool/ApktoolProvider.ts b/src/apktool/ApktoolProvider.ts index 682e2f2ea5..1e1f187c0a 100644 --- a/src/apktool/ApktoolProvider.ts +++ b/src/apktool/ApktoolProvider.ts @@ -117,13 +117,20 @@ const readBoundedText = async ( throwIfCancelled(operation, signal); const truncated = length > budget; // A prefix may end within a UTF-8 character; omit those trailing bytes. - const decoder = new TextDecoder("utf-8"); - return { - text: decoder.decode(bytes.subarray(0, Math.min(length, budget)), { - stream: truncated, - }), - truncated, - }; + try { + const decoder = new TextDecoder("utf-8", { fatal: true }); + return { + text: decoder.decode(bytes.subarray(0, Math.min(length, budget)), { + stream: truncated, + }), + truncated, + }; + } catch (cause: unknown) { + throw new AnalysisProtocolError( + `apktool produced invalid UTF-8 in ${path}`, + { cause }, + ); + } } finally { await file.close(); } diff --git a/src/filesystem/RegularFile.ts b/src/filesystem/RegularFile.ts index a3037c7281..4942e2e467 100644 --- a/src/filesystem/RegularFile.ts +++ b/src/filesystem/RegularFile.ts @@ -1,6 +1,6 @@ import { constants } from "node:fs"; import type { BigIntStats, Stats } from "node:fs"; -import { open, type FileHandle } from "node:fs/promises"; +import { lstat, open, type FileHandle } from "node:fs/promises"; import { OwnedFileHandle } from "./OwnedFileHandle.js"; @@ -74,6 +74,16 @@ export const openRegularFile = async ( options.signal?.throwIfAborted(); if (!metadata.isFile()) throw new NonRegularFileReadError(path, metadata.isDirectory()); + // O_NOFOLLOW is unavailable on Windows. Recheck the name only on hosts + // where open could have followed a symlink, while the descriptor is owned. + if (options.symlinks === "reject" && (constants.O_NOFOLLOW ?? 0) === 0) { + const named = await lstat(path); + options.signal?.throwIfAborted(); + if (named.isSymbolicLink()) + throw new NonRegularFileReadError(path, false); + if (!sameRegularFileState(metadata, named)) + throw new RegularFileChangedError(path); + } return handle; } catch (cause: unknown) { try { diff --git a/tests/process-global/regularFileNoFollowFallback.test.ts b/tests/process-global/regularFileNoFollowFallback.test.ts new file mode 100644 index 0000000000..44ce66c7c0 --- /dev/null +++ b/tests/process-global/regularFileNoFollowFallback.test.ts @@ -0,0 +1,55 @@ +import { readFile, symlink } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect, it, vi } from "vitest"; + +vi.mock("node:fs", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + constants: { ...original.constants, O_NOFOLLOW: 0 }, + }; +}); + +import { openRegularFile } from "../../src/filesystem/RegularFile.js"; +import { ZipArtifactReader } from "../../src/artifacts/ZipArtifactReader.js"; +import { writeOrderedZip } from "../fixtures/artifactEntryOrder.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +it("rejects a symlink when O_NOFOLLOW is unavailable and still follows when requested", async () => { + const root = await createTestTempDirectory("rea-no-nofollow-"); + const target = join(root, "target.zip"); + const link = join(root, "selected.zip"); + await writeOrderedZip(target, ["member.bin"]); + await symlink(target, link); + + const reader = new ZipArtifactReader(target, "zip"); + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + expect(entries.map(({ path }) => path)).toEqual(["member.bin"]); + } finally { + await reader.close(); + } + + const linkedReader = new ZipArtifactReader(link, "zip"); + try { + await expect(async () => { + for await (const _entry of linkedReader.entries()) { + // Consume through the public reader boundary. + } + }).rejects.toMatchObject({ + name: "ArtifactReaderFailure", + reason: "format", + }); + } finally { + await linkedReader.close(); + } + + const followed = await openRegularFile(link, { symlinks: "follow" }); + try { + await expect(readFile(followed)).resolves.toEqual(await readFile(target)); + } finally { + await followed.close(); + } +});