From 9a2aff2beaa0def1ae4917fef73e9e1a6a945338 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 14 Nov 2026 23:04:11 +0800 Subject: [PATCH] fix(artifacts): refuse an ASAR header that declares one member name twice (#10552) JSON.parse keeps only the last value for a repeated name, so an ASAR header that declared a path twice listed one member, hid the other and still reported complete coverage. Add findDuplicateJsonMember, an iterative scan of validated JSON text that does not recurse, so the deeply nested headers REA already admits stay readable. readAsarHeader now refuses a repeated member name as a malformed index, the same way the listing refuses unsafe path components. --- src/artifacts/AsarHeader.test.ts | 24 ++++++++++ src/artifacts/AsarHeader.ts | 20 +++++++- src/domain/jsonDuplicateMember.test.ts | 29 ++++++++++++ src/domain/jsonDuplicateMember.ts | 63 ++++++++++++++++++++++++++ 4 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 src/domain/jsonDuplicateMember.test.ts create mode 100644 src/domain/jsonDuplicateMember.ts 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; +};