Sanitize file names taken from server metadata (closes #9)
check / check (push) Successful in 37s
check / check (push) Successful in 37s
A file title or album name decrypted from server data could name a path outside the chosen directory (`../../.ssh/authorized_keys`). One module, src/filename.ts, now makes such names safe for `quak get`/`get-thumb` without `--out`, downloadFile/downloadThumbnail without outPath, and the backup and metadata backup trees. Originals-cache extensions are limited to letters and digits. A user-supplied path is still used as is. decryptFile reads a missing or non-string title as "" and rejects metadata that is not a JSON object. Model: opus-5-5
This commit was merged in pull request #78.
This commit is contained in:
@@ -49,6 +49,7 @@
|
||||
|
||||
import {
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
readdirSync,
|
||||
readFileSync,
|
||||
rmSync,
|
||||
@@ -501,6 +502,22 @@ const entryPoints = [
|
||||
{ name: "downloadThumbnail", download: downloadThumbnail },
|
||||
];
|
||||
|
||||
// With no `outPath`, the destination is named after `metadata.title`, relative
|
||||
// to the working directory. Such tests run inside a temporary directory:
|
||||
// `make check` must not create files in the repo root.
|
||||
const inDirectory = async <T>(
|
||||
dir: string,
|
||||
run: () => Promise<T>,
|
||||
): Promise<T> => {
|
||||
const previous = process.cwd();
|
||||
process.chdir(dir);
|
||||
try {
|
||||
return await run();
|
||||
} finally {
|
||||
process.chdir(previous);
|
||||
}
|
||||
};
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tests
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -536,26 +553,59 @@ describe("downloadFile", () => {
|
||||
});
|
||||
|
||||
it("uses metadata.title as filename when outPath is omitted", async () => {
|
||||
// With no `outPath`, the destination is `metadata.title`, used
|
||||
// verbatim as a path. The title here is therefore given inside the
|
||||
// test's temporary directory: a bare relative name would resolve
|
||||
// against the process working directory, i.e. the repo root, and
|
||||
// `make check` must not create files in the repo — a failure between
|
||||
// the write and any cleanup would leave one behind.
|
||||
const plaintext = new Uint8Array([1, 2, 3]);
|
||||
const key = sodium.crypto_secretstream_xchacha20poly1305_keygen();
|
||||
const { header, ciphertext } = encryptFileBody(plaintext, key);
|
||||
const thumbPush =
|
||||
sodium.crypto_secretstream_xchacha20poly1305_init_push(key);
|
||||
const file = buildMockEnteFile(key, header, thumbPush.header);
|
||||
const titlePath = join(testDir, "fallback-name.png");
|
||||
file.metadata.title = titlePath;
|
||||
file.metadata.title = "fallback-name.png";
|
||||
const dir = mkdtempSync(join(testDir, "title-"));
|
||||
|
||||
const api = new ApiClient({ fetch: mockFetchForBody(ciphertext) });
|
||||
const result = await downloadFile(api, file);
|
||||
const result = await inDirectory(dir, () => downloadFile(api, file));
|
||||
|
||||
expect(result.path).toBe(titlePath);
|
||||
expect(readFileSync(result.path)).toEqual(Buffer.from(plaintext));
|
||||
expect(result.path).toBe("fallback-name.png");
|
||||
expect(readFileSync(join(dir, "fallback-name.png"))).toEqual(
|
||||
Buffer.from(plaintext),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps a hostile title inside the working directory", async () => {
|
||||
// The server controls the title. `../escaped.png` must not write to
|
||||
// the parent directory; it becomes one file name in the current one.
|
||||
const { api, file } = fixtureFor(
|
||||
multiChunkKey,
|
||||
multiChunk.header,
|
||||
multiChunk.body,
|
||||
);
|
||||
file.metadata.title = "../escaped.png";
|
||||
const parent = mkdtempSync(join(testDir, "hostile-"));
|
||||
const dir = join(parent, "cwd");
|
||||
mkdirSync(dir);
|
||||
|
||||
const result = await inDirectory(dir, () => downloadFile(api, file));
|
||||
|
||||
expect(result.path).toBe("__escaped.png");
|
||||
expect(readdirSync(dir)).toEqual(["__escaped.png"]);
|
||||
expect(readdirSync(parent)).toEqual(["cwd"]);
|
||||
});
|
||||
|
||||
it("uses an explicit outPath verbatim, even one with ..", async () => {
|
||||
// The caller is trusted: its path is not sanitized.
|
||||
const { api, file } = fixtureFor(
|
||||
multiChunkKey,
|
||||
multiChunk.header,
|
||||
multiChunk.body,
|
||||
);
|
||||
const dir = mkdtempSync(join(testDir, "explicit-"));
|
||||
mkdirSync(join(dir, "sub"));
|
||||
const outPath = join(dir, "sub", "..", "explicit.bin");
|
||||
|
||||
const result = await downloadFile(api, file, outPath);
|
||||
|
||||
expect(result.path).toBe(outPath);
|
||||
expect(existsSync(join(dir, "explicit.bin"))).toBe(true);
|
||||
});
|
||||
|
||||
it("handles a larger single-chunk file (random binary payload)", async () => {
|
||||
@@ -617,6 +667,23 @@ describe("downloadThumbnail", () => {
|
||||
expect(result).toEqual({ path: outPath, bytesWritten: 4 });
|
||||
expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext));
|
||||
});
|
||||
|
||||
it("names the thumbnail thumb_ plus the sanitized title", async () => {
|
||||
const { api, file } = fixtureFor(
|
||||
multiChunkKey,
|
||||
multiChunk.header,
|
||||
multiChunk.body,
|
||||
);
|
||||
file.metadata.title = "/etc/passwd";
|
||||
const dir = mkdtempSync(join(testDir, "thumb-title-"));
|
||||
|
||||
const result = await inDirectory(dir, () =>
|
||||
downloadThumbnail(api, file),
|
||||
);
|
||||
|
||||
expect(result.path).toBe("thumb__etc_passwd");
|
||||
expect(readdirSync(dir)).toEqual(["thumb__etc_passwd"]);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user