|
| 1 | +/** |
| 2 | + * GHSA-8hg8-63c5-gwmx — `nesting: true` bypasses `require: false` |
| 3 | + * |
| 4 | + * ## Vulnerability |
| 5 | + * `new NodeVM({ nesting: true, require: false })` constructed a permissive |
| 6 | + * resolver containing the `NESTING_OVERRIDE` builtin (which exposes `vm2`) |
| 7 | + * despite `require: false`. Sandbox code could `require('vm2')`, construct |
| 8 | + * an inner `NodeVM` with attacker-chosen `require` config, and load |
| 9 | + * `child_process` for full host RCE. The mental-model mismatch — developer |
| 10 | + * sets `require: false` to lock down modules, then enables `nesting: true` |
| 11 | + * for legitimate child-VM use — silently produces an unsandboxed config. |
| 12 | + * |
| 13 | + * ## Fix |
| 14 | + * `NodeVM` constructor throws `VMError` immediately when both `nesting: true` |
| 15 | + * and `require: false` are set explicitly. Forces the developer to make a |
| 16 | + * deliberate choice: drop `nesting`, or replace `require: false` with an |
| 17 | + * explicit `require` config. Same shape as the cp6g eager FileSystem probe. |
| 18 | + * |
| 19 | + * ## Out of scope |
| 20 | + * `nesting: true` is fundamentally an escape hatch (sandbox can `require('vm2')` |
| 21 | + * and construct inner NodeVMs unconstrained by the outer config). This fix |
| 22 | + * closes the specific contradictory-config trap; the broader escape-hatch |
| 23 | + * nature of `nesting: true` is now documented prominently in README § |
| 24 | + * "`nesting: true` is an escape hatch". |
| 25 | + */ |
| 26 | + |
| 27 | +'use strict'; |
| 28 | + |
| 29 | +const assert = require('assert'); |
| 30 | +const { NodeVM, VMError } = require('../../../lib/main.js'); |
| 31 | + |
| 32 | +describe('GHSA-8hg8-63c5-gwmx — nesting: true bypasses require: false', () => { |
| 33 | + |
| 34 | + it('rejects { nesting: true, require: false } at construction', () => { |
| 35 | + assert.throws( |
| 36 | + () => new NodeVM({ nesting: true, require: false }), |
| 37 | + err => err instanceof VMError |
| 38 | + && /nesting/.test(err.message) |
| 39 | + && /require/.test(err.message) |
| 40 | + && /GHSA-8hg8-63c5-gwmx/.test(err.message), |
| 41 | + 'construction should fail with a VMError citing nesting, require, and the advisory' |
| 42 | + ); |
| 43 | + }); |
| 44 | + |
| 45 | + it('original PoC config is blocked at construction (cannot reach require(\'vm2\'))', () => { |
| 46 | + // Without the fix, this would succeed and the inner VM would execute |
| 47 | + // child_process.execSync('id'). With the fix, construction throws |
| 48 | + // before vm.run is ever called. |
| 49 | + assert.throws(() => { |
| 50 | + const vm = new NodeVM({ nesting: true, require: false }); |
| 51 | + vm.run(` |
| 52 | + const { NodeVM: NVM } = require('vm2'); |
| 53 | + const inner = new NVM({ require: { builtin: ['child_process'] } }); |
| 54 | + module.exports = inner.run('module.exports = require("child_process").execSync("id").toString()'); |
| 55 | + `); |
| 56 | + }, err => err instanceof VMError && /GHSA-8hg8-63c5-gwmx/.test(err.message)); |
| 57 | + }); |
| 58 | + |
| 59 | + it('accepts { nesting: true, require: { builtin: [] } } (explicit empty allowlist)', () => { |
| 60 | + // Legitimate use: nesting enabled, no other host modules. Developer |
| 61 | + // has explicitly acknowledged that vm2 will be requireable. |
| 62 | + assert.doesNotThrow(() => new NodeVM({ nesting: true, require: { builtin: [] } })); |
| 63 | + }); |
| 64 | + |
| 65 | + it('accepts { nesting: true } alone (default require — escape-hatch use, documented)', () => { |
| 66 | + // Bare `nesting: true` continues to work as documented. The README |
| 67 | + // "`nesting: true` is an escape hatch" section explains the trade-off. |
| 68 | + // Not closed here (would require Option C constraint propagation — |
| 69 | + // out of scope for 3.11.1). This regression test ensures the narrow |
| 70 | + // fix doesn't accidentally break the bare-nesting case. |
| 71 | + assert.doesNotThrow(() => new NodeVM({ nesting: true })); |
| 72 | + }); |
| 73 | + |
| 74 | + it('accepts { require: false } alone (no nesting — deny all requires)', () => { |
| 75 | + // Existing behavior: require: false without nesting is fine — sandbox |
| 76 | + // truly cannot require anything. Regression guard. |
| 77 | + assert.doesNotThrow(() => new NodeVM({ require: false })); |
| 78 | + }); |
| 79 | + |
| 80 | + it('accepts { } (default config — no nesting, no require)', () => { |
| 81 | + // Regression guard for the most common case. |
| 82 | + assert.doesNotThrow(() => new NodeVM()); |
| 83 | + }); |
| 84 | + |
| 85 | +}); |
0 commit comments