mirror of
https://github.com/Kong/insomnia.git
synced 2026-08-02 02:37:25 -04:00
fix(templating): clamp discovered hook counts to prevent an allocation DoS
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.
This commit is contained in:
@@ -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')),
|
||||
|
||||
@@ -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),',
|
||||
|
||||
@@ -12,7 +12,10 @@ const noBridge: HostBridge = async path => {
|
||||
throw new Error(`unexpected bridge call: ${path}`);
|
||||
};
|
||||
|
||||
const envelope = (moduleFiles: Record<string, string>, grantedModules: string[] = ['path', 'crypto']): ContextEnvelope => ({
|
||||
const envelope = (
|
||||
moduleFiles: Record<string, string>,
|
||||
grantedModules: string[] = ['path', 'crypto'],
|
||||
): ContextEnvelope => ({
|
||||
args: [],
|
||||
context: {},
|
||||
meta: {},
|
||||
@@ -26,7 +29,10 @@ const envelope = (moduleFiles: Record<string, string>, grantedModules: string[]
|
||||
entryModuleKey: 'index.js',
|
||||
});
|
||||
|
||||
const discover = async (moduleFiles: Record<string, string>, grantedModules?: string[]): Promise<PluginExportManifest> => {
|
||||
const discover = async (
|
||||
moduleFiles: Record<string, string>,
|
||||
grantedModules?: string[],
|
||||
): Promise<PluginExportManifest> => {
|
||||
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/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user