fix(filesystem): enforce symlink and decoded-text admission

This commit is contained in:
morluto
2026-10-13 13:39:57 +08:00
parent 2187e16f14
commit ec0b081371
4 changed files with 127 additions and 9 deletions
+47 -1
View File
@@ -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 =
'<resources><string name="empty"/><string name="raw"> A &amp; B </string><string name="styled">Hello <b>world</b></string></resources>';
+14 -7
View File
@@ -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();
}
+11 -1
View File
@@ -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 {
@@ -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<typeof import("node:fs")>();
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();
}
});