diff --git a/src/artifacts/AsarHeader.test.ts b/src/artifacts/AsarHeader.test.ts index 1f6ae354e6..673ca50efa 100644 --- a/src/artifacts/AsarHeader.test.ts +++ b/src/artifacts/AsarHeader.test.ts @@ -81,6 +81,30 @@ it("rejects a signature whose declared header exceeds the observed archive", () }); }); +it("refuses a header that declares one member name twice", async () => { + const archive = asarHeaderBytes( + '{"files":{"a.js":{"size":10,"offset":"0"},"a.js":{"size":5,"offset":"10"}}}', + ); + const handle = { + stat: async () => ({ isFile: () => true, size: archive.length }) as Stats, + read: async ( + target: Buffer, + offset: number, + length: number, + position: number, + ) => { + const bytesRead = Math.min(length, archive.length - position); + archive.copy(target, offset, position, position + bytesRead); + return { bytesRead, buffer: target }; + }, + } as unknown as Parameters[0]; + const extents = await admitAsarHeader(handle); + await expect(readAsarHeader(handle, extents)).rejects.toMatchObject({ + reason: "format", + message: expect.stringContaining('"a.js"'), + }); +}); + it("decodes admitted JSON across short reads without consuming padding", async () => { const archive = asarHeaderBytes('{"files":{"�":{}}}', { padding: Buffer.from([0xff]), diff --git a/src/artifacts/AsarHeader.ts b/src/artifacts/AsarHeader.ts index 5a40c9b528..d52cf64abc 100644 --- a/src/artifacts/AsarHeader.ts +++ b/src/artifacts/AsarHeader.ts @@ -1,6 +1,7 @@ import type { Stats } from "node:fs"; import type { FileHandle } from "node:fs/promises"; import { readFileHandleRange } from "../filesystem/readFileHandleRange.js"; +import { findDuplicateJsonMember } from "../domain/jsonDuplicateMember.js"; import { assertArtifactReadActive } from "./ArtifactFileOperation.js"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; @@ -444,5 +445,22 @@ export const readAsarHeader = async ( { cause }, ); } - return JSON.parse(json) as unknown; + const header = JSON.parse(json) as unknown; + rejectDuplicateAsarHeaderMembers(json); + return header; +}; + +/** + * `JSON.parse` keeps only the last of repeated member names, so a second + * declaration of a path would hide the first from the listing while coverage + * still read as complete. Treat a repeated name as a malformed index, like the + * unsafe path components the listing already refuses. + */ +const rejectDuplicateAsarHeaderMembers = (json: string): void => { + const duplicate = findDuplicateJsonMember(json); + if (duplicate !== undefined) + throw new ArtifactReaderFailure( + "format", + `ASAR JSON header declares member ${JSON.stringify(duplicate.name)} more than once in one object (offset ${String(duplicate.offset)})`, + ); }; diff --git a/src/domain/jsonDuplicateMember.test.ts b/src/domain/jsonDuplicateMember.test.ts new file mode 100644 index 0000000000..f01e26b405 --- /dev/null +++ b/src/domain/jsonDuplicateMember.test.ts @@ -0,0 +1,29 @@ +import { expect, it } from "vitest"; + +import { findDuplicateJsonMember } from "./jsonDuplicateMember.js"; + +it.each([ + ['{"a":1,"a":2}', { name: "a", offset: 7 }], + ['{"x":{"a":1,"b":{"a":2},"a":3}}', { name: "a", offset: 24 }], + ['{"a":1,"\\u0061":2}', { name: "a", offset: 7 }], + ['[{"k":"k"},{"k":["k","k"],"k":0}]', { name: "k", offset: 26 }], +])("finds the repeated name in %s", (text, expected) => { + expect(findDuplicateJsonMember(text)).toEqual(expected); +}); + +it.each([ + '{"a":"a","b":"a"}', + '{"a":{"a":{"a":1}}}', + '["a","a",{"a":1},{"a":2}]', + '{"a\\"b":1,"a":2,"b":"\\\\"}', + '"a"', + "{}", +])("accepts distinct names in %s", (text) => { + expect(findDuplicateJsonMember(text)).toBeUndefined(); +}); + +it("scans nesting deeper than the call stack allows", () => { + const depth = 200_000; + const text = `${'{"d":'.repeat(depth)}{"a":1,"a":2}${"}".repeat(depth)}`; + expect(findDuplicateJsonMember(text)).toMatchObject({ name: "a" }); +}); diff --git a/src/domain/jsonDuplicateMember.ts b/src/domain/jsonDuplicateMember.ts new file mode 100644 index 0000000000..8029cbb75c --- /dev/null +++ b/src/domain/jsonDuplicateMember.ts @@ -0,0 +1,63 @@ +/** A member name that occurs more than once in one JSON object. */ +export interface JsonDuplicateMember { + readonly name: string; + /** UTF-16 offset of the repeated name's opening quote. */ + readonly offset: number; +} + +const QUOTE = 0x22; +const BACKSLASH = 0x5c; + +/** + * Find the first object member name that repeats within its object. + * + * `JSON.parse` keeps only the last value for a repeated name, so callers that + * must not lose a declaration check the source text with this. The text must + * already be valid JSON; the scan is iterative, so nesting depth is bounded by + * memory rather than the call stack. + */ +export const findDuplicateJsonMember = ( + text: string, +): JsonDuplicateMember | undefined => { + // One entry per open container: the names seen so far for an object, or + // null for an array. `expectName` marks the next string as a member name. + const open: (Set | null)[] = []; + let expectName = false; + for (let index = 0; index < text.length; index++) { + const code = text.charCodeAt(index); + if (code === QUOTE) { + const start = index; + for (index++; text.charCodeAt(index) !== QUOTE; index++) + if (text.charCodeAt(index) === BACKSLASH) index++; + if (!expectName) continue; + expectName = false; + const names = open.at(-1); + if (names === null || names === undefined) continue; + const raw = text.slice(start, index + 1); + const name = raw.includes("\\") + ? (JSON.parse(raw) as string) + : raw.slice(1, -1); + if (names.has(name)) return { name, offset: start }; + names.add(name); + continue; + } + switch (text[index]) { + case "{": + open.push(new Set()); + expectName = true; + break; + case "[": + open.push(null); + break; + case "}": + case "]": + open.pop(); + expectName = false; + break; + case ",": + expectName = open.at(-1) instanceof Set; + break; + } + } + return undefined; +};