diff --git a/src/domain/javascript/electronIntrinsicCapture.test.ts b/src/domain/javascript/electronIntrinsicCapture.test.ts index 13e2b5a7c7..2679dd55f8 100644 --- a/src/domain/javascript/electronIntrinsicCapture.test.ts +++ b/src/domain/javascript/electronIntrinsicCapture.test.ts @@ -11,6 +11,26 @@ const WINDOW = `let electron = require("electron"); electron = __toESM(electron, 1); new electron.BrowserWindow({});`; describe("captured intrinsic invocation reads", () => { + it("evaluates assignment operands before recording their writes", () => { + const result = analyzeJavaScriptStaticSource(` + const define = Object.defineProperty; + const metadata = {}; + Object.defineProperty = (Object.defineProperty(metadata, "x", {value:1}), replacement); + ${CAPTURED_INTEROP} ${WINDOW} + `); + expect(result.electron.browser_windows).toHaveLength(1); + }); + + it("retains whole-object mutations at a captured function's later member read", () => { + const result = analyzeJavaScriptStaticSource(` + const define = Object.defineProperty; + Object.assign(define, {call: replacement}); + const metadata = {}; define.call(null, metadata, "x", {value:1}); + ${CAPTURED_INTEROP} ${WINDOW} + `); + expect(result.electron.browser_windows).toEqual([]); + }); + it.each([ '(0, Object.defineProperty.call)(null,Object,"create",{value:replacement});', '(0, Object.defineProperty.apply)(null,[Object,"create",{value:replacement}]);', diff --git a/src/domain/javascript/javascriptElectronMemberWrites.ts b/src/domain/javascript/javascriptElectronMemberWrites.ts index 49f8dd732b..bfa4d77fc5 100644 --- a/src/domain/javascript/javascriptElectronMemberWrites.ts +++ b/src/domain/javascript/javascriptElectronMemberWrites.ts @@ -52,6 +52,24 @@ export interface JavaScriptIntrinsicInvocation { | undefined; } +/** Distinguish replacing a value from mutating members of the captured object. */ +export type JavaScriptIdentityWriteKind = + | "replace-value" + | "mutate-object" + | "opaque"; + +/** The source boundary after the operands required for this effect are evaluated. */ +export const javaScriptIdentityWriteOffset = (node: t.Node): number => + (t.isForOfStatement(node) || t.isForInStatement(node) + ? node.right.end + : t.isCallExpression(node) || + t.isOptionalCallExpression(node) || + t.isAssignmentExpression(node) || + t.isUpdateExpression(node) || + t.isUnaryExpression(node, { operator: "delete" }) + ? node.end + : node.start) ?? Infinity; + /** * Index writes through possible lexical object identities. Resolve the complete * object before appending the written key: self-reference properties can denote @@ -68,7 +86,11 @@ export function* collectJavaScriptIdentityWritesSteps( ) => JavaScriptIntrinsicInvocation | undefined, staticString: (node: t.Node | undefined) => string | undefined, freshObject: (node: t.Node) => boolean, - retain: (node: t.Node, path: readonly (string | null)[]) => void, + retain: ( + node: t.Node, + path: readonly (string | null)[], + kind: JavaScriptIdentityWriteKind, + ) => void, ): Generator { const objectMutators = new Set([ "defineProperty", @@ -83,87 +105,105 @@ export function* collectJavaScriptIdentityWritesSteps( "setPrototypeOf", ]); const keyedMutators = new Set(["set", "deleteProperty", "defineProperty"]); - yield* traverseJavaScriptAstSteps(program, { - enter: (node) => { - const target = t.isAssignmentExpression(node) - ? node.left - : t.isUpdateExpression(node) - ? node.argument - : t.isUnaryExpression(node, { operator: "delete" }) - ? node.argument - : t.isForOfStatement(node) || t.isForInStatement(node) - ? node.left - : undefined; - if (target === undefined) return; - for (const leaf of semanticWriteTargets(target)) { - if (t.isIdentifier(leaf)) { - for (const identity of origin(leaf, "binding")) - retain(node, identity); - continue; + const retainCallEffects = ( + node: t.CallExpression | t.OptionalCallExpression, + ): void => { + const invocation = intrinsicInvocation(node); + if (invocation?.kind === "opaque") { + for (const scope of origin(node, "opaque-call")) + retain(node, scope, "opaque"); + return; + } + const callee = invocation?.path; + const arguments_ = invocation?.arguments; + const method = callee?.at(-1); + const owner = callee?.length === 3 ? callee[1] : undefined; + if ( + (owner === "Object" && objectMutators.has(method ?? "")) || + (owner === "Reflect" && reflectMutators.has(method ?? "")) + ) { + // Reflect.set can write through its explicit receiver as well as + // dispatching through its target. Both identities need examination. + const targets = + owner === "Reflect" && + method === "set" && + arguments_ !== undefined && + (arguments_.length >= 4 || arguments_.some(t.isSpreadElement)) + ? [ + arguments_[0], + arguments_.some(t.isSpreadElement) ? undefined : arguments_[3], + ] + : [arguments_?.[0]]; + for (const target of targets) { + // A known mutator with an opaque argument list can overwrite any + // captured intrinsic. Do not turn an unexamined target into proof. + if (target === undefined || t.isSpreadElement(target)) { + for (const scope of origin(node, "opaque-call")) + retain(node, scope, "opaque"); } - const member = unwrapJavaScriptExpression(leaf).node; - if ( - !t.isMemberExpression(member) && - !t.isOptionalMemberExpression(member) - ) - continue; - const key = semanticStaticPropertyKey(member.property, member.computed); - for (const identity of origin(member.object, "member")) - retain(node, [...identity, key]); - } - }, - exit: (node) => { - if (t.isCallExpression(node) || t.isOptionalCallExpression(node)) { - const invocation = intrinsicInvocation(node); - if (invocation?.kind === "opaque") { - for (const scope of origin(node, "opaque-call")) retain(node, scope); - return; - } - const callee = invocation?.path; - const arguments_ = invocation?.arguments; - const method = callee?.at(-1); - const owner = callee?.length === 3 ? callee[1] : undefined; - if ( - (owner === "Object" && objectMutators.has(method ?? "")) || - (owner === "Reflect" && reflectMutators.has(method ?? "")) - ) { - // Reflect.set can write through its explicit receiver as well as - // dispatching through its target. Both identities need examination. - const targets = - owner === "Reflect" && - method === "set" && - arguments_ !== undefined && - (arguments_.length >= 4 || arguments_.some(t.isSpreadElement)) - ? [ - arguments_[0], - arguments_.some(t.isSpreadElement) - ? undefined - : arguments_[3], - ] - : [arguments_?.[0]]; - for (const target of targets) { - // A known mutator with an opaque argument list can overwrite any - // captured intrinsic. Do not turn an unexamined target into proof. - if (target === undefined || t.isSpreadElement(target)) { - for (const scope of origin(node, "opaque-call")) - retain(node, scope); - } - if (t.isNode(target) && !t.isSpreadElement(target)) { - const identities = origin(target, "member"); - if (identities.length > 0) { - const key = keyedMutators.has(method ?? "") - ? [staticString(arguments_?.[1]) ?? null] - : []; - for (const identity of identities) - retain(node, [...identity, ...key]); - } else if (!freshObject(target)) { - for (const scope of origin(node, "opaque-call")) - retain(node, scope); - } - } + if (t.isNode(target) && !t.isSpreadElement(target)) { + const identities = origin(target, "member"); + if (identities.length > 0) { + const key = keyedMutators.has(method ?? "") + ? [staticString(arguments_?.[1]) ?? null] + : []; + for (const identity of identities) + retain( + node, + [...identity, ...key], + keyedMutators.has(method ?? "") + ? "replace-value" + : "mutate-object", + ); + } else if (!freshObject(target)) { + for (const scope of origin(node, "opaque-call")) + retain(node, scope, "opaque"); } } } + } + }; + const retainTargets = (node: t.Node): void => { + const target = t.isAssignmentExpression(node) + ? node.left + : t.isUpdateExpression(node) + ? node.argument + : t.isUnaryExpression(node, { operator: "delete" }) + ? node.argument + : t.isForOfStatement(node) || t.isForInStatement(node) + ? node.left + : undefined; + if (target === undefined) return; + for (const leaf of semanticWriteTargets(target)) { + if (t.isIdentifier(leaf)) { + for (const identity of origin(leaf, "binding")) + retain(node, identity, "replace-value"); + continue; + } + const member = unwrapJavaScriptExpression(leaf).node; + if ( + !t.isMemberExpression(member) && + !t.isOptionalMemberExpression(member) + ) + continue; + const key = semanticStaticPropertyKey(member.property, member.computed); + for (const identity of origin(member.object, "member")) + retain(node, [...identity, key], "replace-value"); + } + }; + const loopBodies = new WeakMap(); + yield* traverseJavaScriptAstSteps(program, { + enter: (node) => { + if (t.isForOfStatement(node) || t.isForInStatement(node)) + loopBodies.set(node.body, node); + const loop = loopBodies.get(node); + if (loop !== undefined) retainTargets(loop); + }, + exit: (node) => { + if (t.isCallExpression(node) || t.isOptionalCallExpression(node)) + retainCallEffects(node); + else if (!t.isForOfStatement(node) && !t.isForInStatement(node)) + retainTargets(node); }, }); } diff --git a/src/domain/javascript/javascriptModuleInterop.ts b/src/domain/javascript/javascriptModuleInterop.ts index f573324f43..1506e462bf 100644 --- a/src/domain/javascript/javascriptModuleInterop.ts +++ b/src/domain/javascript/javascriptModuleInterop.ts @@ -17,6 +17,8 @@ import { } from "./javascriptSemanticTraversal.js"; import { electronMemberPath, + javaScriptIdentityWriteOffset, + type JavaScriptIdentityWriteKind, type JavaScriptIntrinsicInvocation, } from "./javascriptElectronMemberWrites.js"; import { @@ -99,7 +101,17 @@ export const createSemanticModuleInterop = ( readonly scope: string; readonly offset: number; readonly path: readonly (string | null)[]; + readonly kind: JavaScriptIdentityWriteKind; }[] = []; + const invocationMemberWriteCovers = ( + write: (typeof writes)[number], + read: readonly string[], + ): boolean => + (write.kind === "opaque" || + (write.kind === "mutate-object" && + write.path.length === read.length - 1) || + (write.kind === "replace-value" && write.path.length === read.length)) && + intrinsicWriteCovers(write.path, read); const invalidatedBindings = new Map(); const invalidatedCalls = new WeakMap(); const opaqueMutationPath = (node: t.Node): readonly string[] => { @@ -293,9 +305,8 @@ export const createSemanticModuleInterop = ( (write) => (write.scope === scope || write.scope === "intrinsic") && write.offset < offset && - ((write.path.length >= currentPath.length && - intrinsicWriteCovers(write.path, currentPath)) || - intrinsicWriteCovers(write.path, [ + (invocationMemberWriteCovers(write, currentPath) || + invocationMemberWriteCovers(write, [ "intrinsic", "Function", "prototype", @@ -711,11 +722,16 @@ export const createSemanticModuleInterop = ( staticString, isFreshObject, opaqueMutationPath, - retainWrite: (node: t.Node, path: readonly (string | null)[]) => { + retainWrite: ( + node: t.Node, + path: readonly (string | null)[], + kind: JavaScriptIdentityWriteKind = "replace-value", + ) => { writes.push({ scope: opaqueMutationPath(node).join("."), - offset: node.start ?? Infinity, + offset: javaScriptIdentityWriteOffset(node), path: path[0] === "opaque-intrinsic" ? ["intrinsic"] : path, + kind, }); }, invalidated: (binding: JavaScriptSemanticBindingState): boolean => { diff --git a/src/domain/javascript/javascriptSemanticAnalysis.ts b/src/domain/javascript/javascriptSemanticAnalysis.ts index f6c6db25d7..60005be44e 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.ts @@ -462,9 +462,9 @@ export function* indexJavaScriptElectronIdentitySteps( interop.intrinsicInvocation, interop.staticString, interop.isFreshObject, - (node, path) => { + (node, path, kind) => { if (path[0] === "electron") writes.retain(path); - else interop.retainWrite(node, path); + else interop.retainWrite(node, path, kind); }, ); collectedWrites = true;