From d3eb7daf0b225fdfcc2d7ba388d40e1cdd2fc103 Mon Sep 17 00:00:00 2001 From: jackkav Date: Sat, 18 Jul 2026 09:45:53 +0200 Subject: [PATCH] fix(templating): clamp discovered hook counts to prevent an allocation DoS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The export manifest reports request/response hooks as a count, and the host builds Array.from({ length: count }) of throw-stubs. A hostile plugin could export a non-array with a huge `.length` (e.g. { length: 1e12 }) so the count crosses to the host and drives an enormous allocation — even though the plugin only ran in the sandbox. Coerce the count to a finite, non-negative integer capped at 1000 inside the sandbox (__describeExports), and clamp again host-side before Array.from as defense in depth. Unit test covers a fake huge-length hooks export. --- packages/insomnia/src/plugins/index.ts | 11 +++++-- .../sandbox/in-sandbox-bootstrap.ts | 8 +++-- .../sandbox/sandbox-export-discovery.test.ts | 33 +++++++++++++++---- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/packages/insomnia/src/plugins/index.ts b/packages/insomnia/src/plugins/index.ts index e1d58f797c..d05820c23e 100644 --- a/packages/insomnia/src/plugins/index.ts +++ b/packages/insomnia/src/plugins/index.ts @@ -87,10 +87,17 @@ function buildUserPluginModuleFromManifest(pluginName: string, manifest: PluginE icon: a.icon, action: notRouted(surface), }); + // The count comes from untrusted sandbox output; clamp to a finite, non-negative, bounded integer + // before allocating so a hostile manifest can't drive a huge Array.from allocation (the sandbox + // clamps too — this is defense in depth). + const hookStubs = (count: number, surface: string) => + Array.from({ length: Number.isFinite(count) && count > 0 ? Math.min(Math.floor(count), 1000) : 0 }, () => + notRouted(surface), + ); return { templateTags: manifest.templateTags.map(t => ({ ...t, run: notRouted('template-tag run()') }) as PluginTemplateTag), - requestHooks: Array.from({ length: manifest.requestHooks }, () => notRouted('request hooks')), - responseHooks: Array.from({ length: manifest.responseHooks }, () => notRouted('response hooks')), + requestHooks: hookStubs(manifest.requestHooks, 'request hooks'), + responseHooks: hookStubs(manifest.responseHooks, 'response hooks'), requestActions: manifest.requestActions.map(toAction('request actions')), requestGroupActions: manifest.requestGroupActions.map(toAction('request group actions')), workspaceActions: manifest.workspaceActions.map(toAction('workspace actions')), diff --git a/packages/insomnia/src/templating/sandbox/in-sandbox-bootstrap.ts b/packages/insomnia/src/templating/sandbox/in-sandbox-bootstrap.ts index ae8a5c202d..0cc9c0530f 100644 --- a/packages/insomnia/src/templating/sandbox/in-sandbox-bootstrap.ts +++ b/packages/insomnia/src/templating/sandbox/in-sandbox-bootstrap.ts @@ -351,10 +351,14 @@ export const IN_SANDBOX_BOOTSTRAP = [ ' var tagDesc = function (t) { return isObj(t) ? { name: t.name, displayName: t.displayName, description: t.description, args: t.args || [] } : null; };', ' var actionDesc = function (a) { return isObj(a) ? { label: a.label, icon: a.icon } : null; };', ' var themeDesc = function (t) { return isObj(t) ? { name: t.name, displayName: t.displayName, theme: t.theme } : null; };', + // Report a hook count, not the array itself — but a plugin could export a non-array with a huge + // `.length` to make the host allocate `Array.from({length})` (DoS). Coerce to a finite, non-negative + // integer and cap it; the host clamps again as defense in depth. + ' var hookCount = function (arr) { var n = arr && arr.length; n = (typeof n === "number" && isFinite(n)) ? Math.floor(n) : 0; if (n < 0) { n = 0; } return n > 1000 ? 1000 : n; };', ' var manifest = {', ' templateTags: mapArr(mod.templateTags, tagDesc),', - ' requestHooks: (mod.requestHooks && mod.requestHooks.length) || 0,', - ' responseHooks: (mod.responseHooks && mod.responseHooks.length) || 0,', + ' requestHooks: hookCount(mod.requestHooks),', + ' responseHooks: hookCount(mod.responseHooks),', ' requestActions: mapArr(mod.requestActions, actionDesc),', ' requestGroupActions: mapArr(mod.requestGroupActions, actionDesc),', ' workspaceActions: mapArr(mod.workspaceActions, actionDesc),', diff --git a/packages/insomnia/src/templating/sandbox/sandbox-export-discovery.test.ts b/packages/insomnia/src/templating/sandbox/sandbox-export-discovery.test.ts index 6c544208d6..c3942c4aa6 100644 --- a/packages/insomnia/src/templating/sandbox/sandbox-export-discovery.test.ts +++ b/packages/insomnia/src/templating/sandbox/sandbox-export-discovery.test.ts @@ -12,7 +12,10 @@ const noBridge: HostBridge = async path => { throw new Error(`unexpected bridge call: ${path}`); }; -const envelope = (moduleFiles: Record, grantedModules: string[] = ['path', 'crypto']): ContextEnvelope => ({ +const envelope = ( + moduleFiles: Record, + grantedModules: string[] = ['path', 'crypto'], +): ContextEnvelope => ({ args: [], context: {}, meta: {}, @@ -26,7 +29,10 @@ const envelope = (moduleFiles: Record, grantedModules: string[] entryModuleKey: 'index.js', }); -const discover = async (moduleFiles: Record, grantedModules?: string[]): Promise => { +const discover = async ( + moduleFiles: Record, + grantedModules?: string[], +): Promise => { const json = await runTagInSandbox({ tagName: '', discover: true, @@ -68,7 +74,22 @@ describe('sandbox export discovery (L1)', () => { 'index.js': "module.exports.templateTags = require('./tags');", 'tags.js': "module.exports = [{ name: 'fromlib', run: function () { return 'x'; } }];", }); - expect(manifest.templateTags).toEqual([{ name: 'fromlib', displayName: undefined, description: undefined, args: [] }]); + expect(manifest.templateTags).toEqual([ + { name: 'fromlib', displayName: undefined, description: undefined, args: [] }, + ]); + }); + + it('clamps a hostile hook count so the host cannot be driven into a huge allocation', async () => { + // A plugin exports a non-array with a giant `.length`; the manifest count must be clamped, not + // passed through, so the host's Array.from({ length }) can't be weaponized into an OOM. + const manifest = await discover({ + 'index.js': ` + module.exports.requestHooks = { length: 1e12 }; + module.exports.responseHooks = [function () {}, function () {}, function () {}]; + module.exports.templateTags = [];`, + }); + expect(manifest.requestHooks).toBe(1000); + expect(manifest.responseHooks).toBe(3); }); it('contains a throwing top-level (surfaced as a rejection, not a host crash)', async () => { @@ -79,8 +100,8 @@ describe('sandbox export discovery (L1)', () => { // A plugin whose top-level does require('child_process') to spawn a process gets nothing: the // sandbox require is default-deny, so discovery fails the same way tag execution would — the // host is never reached. - await expect(discover({ 'index.js': "require('child_process'); module.exports.templateTags = [];" })).rejects.toThrow( - /not permitted by manifest/, - ); + await expect( + discover({ 'index.js': "require('child_process'); module.exports.templateTags = [];" }), + ).rejects.toThrow(/not permitted by manifest/); }); });