diff --git a/src/domain/javascript/electronBundledRequireIdentity.test.ts b/src/domain/javascript/electronBundledRequireIdentity.test.ts index ff1e187fa8..e3f34043ef 100644 --- a/src/domain/javascript/electronBundledRequireIdentity.test.ts +++ b/src/domain/javascript/electronBundledRequireIdentity.test.ts @@ -18,6 +18,18 @@ const CONSUMER = const electronOf = (source: string) => analyzeJavaScriptStaticSource(source).electron; +const expectUnresolved = (source: string) => { + const electron = electronOf(source); + expect(electron.browser_windows).toEqual([]); + // Only direct observations come from this identity; the separate + // wrapper-substitution path keeps its own resolution basis. + expect( + electron.ipc.filter( + ({ resolution_basis: basis }) => basis !== "wrapper-substitution", + ), + ).toEqual([]); +}; + describe("Electron identity through Webpack externals", () => { it("follows the default-export getter produced by webpack n.n", () => { const source = bootstrap( @@ -142,15 +154,7 @@ describe("Electron identity through Webpack externals", () => { bootstrap(EXTERNAL, CONSUMER.replace("n(45840)", "n(45841)")), ], ])("keeps %s unresolved", (_name, source) => { - const electron = electronOf(source); - expect(electron.browser_windows).toEqual([]); - // Only direct observations come from this identity; the separate - // wrapper-substitution path keeps its own resolution basis. - expect( - electron.ipc.filter( - ({ resolution_basis: basis }) => basis !== "wrapper-substitution", - ), - ).toEqual([]); + expectUnresolved(source); }); }); @@ -214,15 +218,7 @@ describe("Webpack externals with a tampered runtime", () => { ), ], ])("keeps %s unresolved", (_name, source) => { - const electron = electronOf(source); - expect(electron.browser_windows).toEqual([]); - // Only direct observations come from this identity; the separate - // wrapper-substitution path keeps its own resolution basis. - expect( - electron.ipc.filter( - ({ resolution_basis: basis }) => basis !== "wrapper-substitution", - ), - ).toEqual([]); + expectUnresolved(source); }); }); @@ -287,15 +283,7 @@ describe("Webpack externals with a tampered loader or module object", () => { ), ], ])("keeps %s unresolved", (_name, source) => { - const electron = electronOf(source); - expect(electron.browser_windows).toEqual([]); - // Only direct observations come from this identity; the separate - // wrapper-substitution path keeps its own resolution basis. - expect( - electron.ipc.filter( - ({ resolution_basis: basis }) => basis !== "wrapper-substitution", - ), - ).toEqual([]); + expectUnresolved(source); }); it.each([ @@ -331,3 +319,60 @@ describe("Webpack externals with a tampered loader or module object", () => { ]); }); }); + +describe("Webpack externals through the loader's completion paths", () => { + const MISS_RETURN = "return m[t](o,o.exports,n),o.exports}"; + const missPath = (tail: string) => + bootstrap(EXTERNAL, CONSUMER).replace(MISS_RETURN, tail); + + // Falling off the end returns undefined, not the dispatched exports. + it.each([ + ["returns only cached exports", missPath("m[t](o,o.exports,n)}")], + [ + "has no returns", + bootstrap(EXTERNAL, CONSUMER).replace( + `var r=c[t];if(void 0!==r)return r.exports;var o=c[t]={exports:{}};${MISS_RETURN}`, + "var o=c[t]={exports:{}};m[t](o,o.exports,n)}", + ), + ], + [ + "returns from one branch", + missPath("m[t](o,o.exports,n);if(t)return o.exports}"), + ], + [ + "returns from a try whose catch falls through", + missPath("try{return m[t](o,o.exports,n),o.exports}catch(x){}}"), + ], + [ + "falls through a catch past a completing finally", + missPath( + "try{return m[t](o,o.exports,n),o.exports}catch(x){}finally{t}}", + ), + ], + [ + "returns another value from a finally", + missPath( + "try{return m[t](o,o.exports,n),o.exports}finally{return local}}", + ), + ], + ])("keeps a loader that %s unresolved", (_name, source) => { + expectUnresolved(source); + }); + + it.each([ + [ + "returns exports from both branches", + missPath( + "m[t](o,o.exports,n);if(t)return o.exports;else return o.exports}", + ), + ], + [ + "returns exports from a finally after a falling-through catch", + missPath("try{m[t](o,o.exports,n)}catch(x){}finally{return o.exports}}"), + ], + ])("still resolves a loader that %s", (_name, source) => { + expect(electronOf(source).browser_windows).toMatchObject([ + { preload_path: "./preload.js" }, + ]); + }); +}); diff --git a/src/domain/javascript/javascriptBootstrapIntegrity.ts b/src/domain/javascript/javascriptBootstrapIntegrity.ts index 1e748df414..69043fe079 100644 --- a/src/domain/javascript/javascriptBootstrapIntegrity.ts +++ b/src/domain/javascript/javascriptBootstrapIntegrity.ts @@ -30,11 +30,12 @@ interface LoaderShape { /** * Prove that `loader(key)` returns the exports of the factory declared at - * `table[key]`: the loader returns only the dispatched module's exports or a - * cache entry it created for that module, the table has only literal keys, - * and the table and cache are referenced only by the loader or through the - * runtime's `loader.m`/`loader.c` exposure. Literal-key writes through that - * exposure replace their slot. Returns undefined when any of this is unproven. + * `table[key]`: every normal completion of the loader returns the dispatched + * module's exports or a cache entry it created for that module, the table has + * only literal keys, and the table and cache are referenced only by the loader + * or through the runtime's `loader.m`/`loader.c` exposure. Literal-key writes + * through that exposure replace their slot. Returns undefined when any of this + * is unproven. */ export const proveJavaScriptBootstrapIntegrity = ( call: t.CallExpression, @@ -74,12 +75,14 @@ const loaderShape = ( const parameterId = t.isIdentifier(parameter) ? bindingOf(state, parameter) : undefined; - // A rewritten key would dispatch a different slot than the caller named. + // A rewritten key would dispatch a different slot than the caller named, + // and falling off the end returns undefined instead of the exports. if ( parameterId === undefined || state.bindingsById.get(parameterId)?.definitions.length !== 1 || loader.async || - loader.generator + loader.generator || + completesNormally(loader.body) ) return undefined; const dispatches = new Set(); @@ -129,6 +132,35 @@ const loaderShape = ( : undefined; }; +/** + * Whether control may reach the end of `statement`. Loops, switches and labels + * can absorb a `break`, so they are assumed to complete. + */ +const completesNormally = (statement: t.Statement): boolean => { + if (t.isReturnStatement(statement) || t.isThrowStatement(statement)) + return false; + if (t.isBlockStatement(statement)) + return statement.body.every(completesNormally); + if (t.isIfStatement(statement)) + return ( + statement.alternate === null || + statement.alternate === undefined || + completesNormally(statement.consequent) || + completesNormally(statement.alternate) + ); + if (t.isTryStatement(statement)) + return ( + (statement.finalizer === null || + statement.finalizer === undefined || + completesNormally(statement.finalizer)) && + (completesNormally(statement.block) || + (statement.handler !== null && + statement.handler !== undefined && + completesNormally(statement.handler.body))) + ); + return true; +}; + /** `table[key](module, ...)` or `table[key].call(thisArg, module, ...)`. */ const tableDispatch = ( call: t.CallExpression, diff --git a/tests/acceptance/applications/webpackLoaderReturnsCli.test.ts b/tests/acceptance/applications/webpackLoaderReturnsCli.test.ts new file mode 100644 index 0000000000..8a12905090 --- /dev/null +++ b/tests/acceptance/applications/webpackLoaderReturnsCli.test.ts @@ -0,0 +1,62 @@ +import { expect } from "vitest"; + +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; +import { writeFixtureFiles } from "../../support/javascriptApplicationFixture.js"; + +const CACHE_HIT = ` + var cached = cache[key]; + if (cached !== undefined) return cached.exports;`; + +const loaderSource = (cacheHit: string, missReturn: string) => `(function () { + var modules = { + 45840: function (module) { module.exports = require("electron"); }, + 1: function (module, exports, n) { new (n(45840).BrowserWindow)({}); } + }, cache = {}; + function n(key) {${cacheHit} + var module = (cache[key] = { exports: {} }); + modules[key](module, module.exports, n);${missReturn} + } + n(1); +})();`; + +cliTest.for([ + [ + "the canonical loader", + loaderSource(CACHE_HIT, "\n return module.exports;"), + 1, + ], + ["a loader that returns only cached exports", loaderSource(CACHE_HIT, ""), 0], + ["a loader without returns", loaderSource("", ""), 0], +] as const)( + "attributes the BrowserWindow to Electron only through %s", + async ([, source, windows], { cli }) => { + const root = await createTestTempDirectory("rea-webpack-loader-returns-"); + await writeFixtureFiles(root, { + "main.js": source, + "package.json": JSON.stringify({ name: "fixture", main: "main.js" }), + }); + const response = await cli.run({ + arguments: ["analyze-javascript-application", root, "--format", "json"], + environment: { REA_LOG_LEVEL: "silent" }, + }); + expect(response.exitCode).toBe(0); + const result = javascriptApplicationAnalysisResultSchema.parse( + parseEvidence(response.json).normalized_result, + ); + expect(result.summary.browser_windows).toBe(windows); + // The literal external stays observed even when the loader is unproven. + expect( + result.graph.nodes.flatMap(({ observations }) => + observations.map(({ properties }) => properties), + ), + ).toContainEqual( + expect.objectContaining({ + reference_kind: "require", + specifier: "electron", + }), + ); + }, +);