Skip to content

Commit 86ab819

Browse files
committed
fix(GHSA-m4wx-m65x-ghrr): widen NESTING_OVERRIDE guard to all input shapes
The prior `nesting === true && !requireOpts` check left two bypass classes that recreate the same NESTING_OVERRIDE-only resolver as the original PoC: truthy non-`true` `nesting` (the override gate is `nesting && NESTING_OVERRIDE`, not strict `=== true`) and truthy non-object `require` (primitives/functions destructure to all-undefined inside `makeResolverFromLegacyOptions`). Replace with a structural guard mirroring the resolver's actual reachability: `nesting` truthy and `requireOpts` must be a non-null object or `Resolver`. Every shape that produces a NESTING_OVERRIDE-only resolver now throws at construction. The explicit-`require`-config escape hatch is preserved. See ATTACKS.md Category 25 and test/ghsa/GHSA-m4wx-m65x-ghrr/.
1 parent e1c48fc commit 86ab819

4 files changed

Lines changed: 203 additions & 73 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ Ten advisories closed. Patch release — no API changes for valid configurations
1111
- **GHSA-v6mx-mf47-r5wg** — host prototype mutation via apply-trap indirection. Sandbox code could reach host prototype-mutating setters (`Object.prototype.__proto__`, `setPrototypeOf`, `defineProperty`, `__defineSetter__`/`__defineGetter__`) through `Function.prototype.{call,apply,bind}` and `Reflect.{apply,construct}` indirection, sever a host intrinsic's prototype chain, and escape via the bridge's `thisEnsureThis` proto-walk fallthrough. Two-layer structural fix in `lib/bridge.js` (apply-trap blocklist + cache check before proto-walk). See ATTACKS.md Category 30 and `test/ghsa/GHSA-v6mx-mf47-r5wg/`.
1212
- **GHSA-q3fm-4wcw-g57x** — Defense Invariant #11 hardening for `defaultSandboxPrepareStackTrace` (second variant of GHSA-9qj6-qjgg-37qq in a different file). The sandbox stack-trace formatter accumulated frames in a sandbox-realm array and `.join`-ed them, so a sandbox-installed setter on `Array.prototype[N]` (or `.join` override) observed bridge-internal state — no host reference reachable today, but one enrichment away from regressing into the GHSA-9qj6 RCE shape. Fixed in `lib/setup-sandbox.js` by folding frames through a primitive string accumulator (no `Array.prototype` slot reachable) and converting `makeCallSiteGetters` to `localReflectDefineProperty` for symmetry. See ATTACKS.md Category 28 Variant B and `test/ghsa/GHSA-q3fm-4wcw-g57x/`.
1313
- **GHSA-76w7-j9cq-rx2j** — Promise species hijack in the `localPromise` swallow tail. The swallow-tail `apply(globalPromisePrototypeThen, this, [...])` call inside `localPromise`'s constructor invoked the cached host `Promise.prototype.then` without first calling `resetPromiseSpecies(this)`, so a sandbox subclass overriding `[Symbol.species]` could redirect the downstream child constructor to a user function and capture V8's internal `(resolve, reject)` capability — delivering a raw host-realm error (RangeError from deep recursion + `e.stack`) to a sandbox collector and reaching the host `Function` constructor via `.constructor.constructor`. One-line fix in `lib/setup-sandbox.js` adds the missing `resetPromiseSpecies(this)` before the swallow-tail call, matching the pattern already used by the `.then`/`.catch`/`Reflect.apply` overrides. See ATTACKS.md Category 31 and `test/ghsa/GHSA-76w7-j9cq-rx2j/`.
14-
- **GHSA-m4wx-m65x-ghrr** — patch bypass of GHSA-8hg8-63c5-gwmx. The original check `options.require === false` only rejected the literal `require: false` shape; omitting `require` entirely left `options.require === undefined`, the check fell through, and the destructuring default `require: requireOpts = false` a few lines down produced the same `requireOpts = false` the original patch existed to block — inner NodeVM construction with attacker-chosen `require` config `child_process` RCE. Structural fix in `lib/nodevm.js`: destructure first, then check `nesting === true && !requireOpts`, so every falsy/omitted shape collapses to the same construction-time `VMError`. The explicit-`require`-config escape hatch is preserved. Supersedes GHSA-8hg8-63c5-gwmx. See ATTACKS.md Category 25 and `test/ghsa/GHSA-m4wx-m65x-ghrr/`.
14+
- **GHSA-m4wx-m65x-ghrr**NodeVM constructor patch bypass of GHSA-8hg8-63c5-gwmx: a truthy `nesting` paired with anything other than a real `require` config object produced a NESTING_OVERRIDE-only resolver → inner NodeVM with attacker-chosen `require``child_process` RCE. Structural fix in `lib/nodevm.js`: destructure first, then reject at construction whenever `nesting` is truthy and `requireOpts` is not a non-null object or `Resolver`. Supersedes GHSA-8hg8-63c5-gwmx. See ATTACKS.md Category 25 and `test/ghsa/GHSA-m4wx-m65x-ghrr/`.
1515
- **GHSA-6j2x-vhqr-qr7q** — sandbox escape via WebAssembly JSPI (Node 24 behind `--experimental-wasm-jspi`, Node 26+ default). `WebAssembly.promising` returns Promise objects whose `[[Prototype]]` chain points directly at the host realm's `Promise.prototype` with no bridge proxy in between, so `p.finally()` reaches host `Promise.prototype.finally`, V8's `SpeciesConstructor` reads an attacker-controlled `p.constructor` getter, and the eventual host-realm rejection is dispatched through the attacker's class with no bridge wrapping — `e.constructor.constructor('return process')()` then evaluates in the host realm. Structural fix in `lib/setup-sandbox.js`: delete `WebAssembly.promising` and `WebAssembly.Suspending` at sandbox bootstrap, mirroring the existing `WebAssembly.JSTag` removal. Adds Defense Invariant #12 (no sandbox-visible object may have a host-realm prototype chain without bridge interposition). See ATTACKS.md Category 33 and `test/ghsa/GHSA-6j2x-vhqr-qr7q/`.
1616
- **GHSA-rp36-8xq3-r6c4** — NodeVM builtin denylist bypass via `process` and `inspector/promises`. The exact-match denylist in `lib/builtin.js` missed two host-passthrough families: `process` (whose `getBuiltinModule(name)` reloads any core module regardless of the embedder's allow/deny configuration) and `inspector/promises` (whose `Session().post('Runtime.evaluate', ...)` evaluates attacker JS in the host realm). Structural fix promotes the check to family-prefix via `isDangerousBuiltin(key)`, strips the `node:` URL prefix, and adds `process` to the dangerous set — enforced at both `BUILTIN_MODULES` source and `addDefaultBuiltin`. Supersedes GHSA-947f-4v7f-x2v8. Adds Defense Invariant #13. See ATTACKS.md Category 21 (extended) and `test/ghsa/GHSA-rp36-8xq3-r6c4/`.
1717
- **GHSA-r9pm-gxmw-wv6p** — NodeVM `builtin: ['*']` wildcard exposed Node's undocumented underscored network builtins (`_http_client`, `_http_server`, the `_http_*` / `_tls_*` / `_stream_*` siblings), letting sandbox code make outbound HTTP requests and open listening sockets even when the documented `-http`/`-https`/`-net`/`-tls` exclusions were used — SSRF-class capability bypass (CVSS 8.6). Structural fix in `lib/builtin.js`: `BUILTIN_MODULES` filter now excludes any name starting with `_`, so `'*'` expands only to documented public builtins; explicit opt-in, `mock`, and `override` paths remain functional. See ATTACKS.md Category 34 and `test/ghsa/GHSA-r9pm-gxmw-wv6p/`.
1818
- **GHSA-9g8x-92q2-p28f** — NodeVM builtin allowlist surfaced four process-wide observability builtins (`diagnostics_channel`, `async_hooks`, `perf_hooks`, `v8`) that read state from the entire host process rather than the sandbox: HTTP `IncomingMessage` headers (incl. auth tokens) via `diagnostics_channel.subscribe`, embedder `AsyncLocalStorage` context via `async_hooks.executionAsyncResource`, embedder `performance.mark` labels via `perf_hooks`, and the full V8 heap via `v8.getHeapSnapshot` / `v8.queryObjects`. Fix in `lib/builtin.js`: extends `DANGEROUS_BUILTINS` with the four names, reusing the existing two-layer enforcement (`BUILTIN_MODULES` filter + `addDefaultBuiltin` rejection, family-prefix and `node:`-normalised via `isDangerousBuiltin`). `mock`/`override` escape hatches preserved. See ATTACKS.md Category 35 and `test/ghsa/GHSA-9g8x-92q2-p28f/`.
1919

2020
### Upgrade notes
2121

22-
- **If you constructed `NodeVM({ nesting: true })` without an explicit `require` config** (including omitting `require` entirely, or setting it to `undefined`/`null`/`0`/`''`), `new NodeVM(...)` now throws (GHSA-m4wx-m65x-ghrr). Either drop `nesting: true`, or pass an explicit `require` config object (e.g. `require: { builtin: [] }`) to acknowledge that vm2 will be requireable from inside the sandbox. The error message is actionable and links to the README hardening section.
22+
- **If you constructed `NodeVM({ nesting: <truthy> })` without an explicit `require` config object**, `new NodeVM(...)` now throws (GHSA-m4wx-m65x-ghrr). This covers every shape that previously silently produced a `vm2`-only resolver: omitting `require` entirely, or setting it to any falsy value (`false`/`undefined`/`null`/`0`/`''`) or any truthy non-object value (`true`/number/string/symbol/function); and also any truthy `nesting` value, not only `nesting: true` (`1`/`'yes'`/`{}`/`[]`/function). Either drop `nesting`, or pass an explicit `require` config object (e.g. `require: { builtin: [] }`) to acknowledge that vm2 will be requireable from inside the sandbox. The error message is actionable and links to the README hardening section.
2323

2424
## [3.11.3]
2525

docs/ATTACKS.md

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1850,24 +1850,23 @@ The race window between the canonicalization syscall and the subsequent loader s
18501850
18511851
---
18521852
1853-
## Attack Category 25: NodeVM `nesting: true` Configuration Trap (NESTING_OVERRIDE-only resolver)
1853+
## Attack Category 25: NodeVM `nesting` Configuration Trap (NESTING_OVERRIDE-only resolver)
18541854
1855-
**Supersedes**: GHSA-8hg8-63c5-gwmx's check on the raw `options.require === false` input, which only fired for the literal `require: false` shape and missed the much commoner "`require` omitted entirely" shape — see GHSA-m4wx-m65x-ghrr.
1855+
**Supersedes**: GHSA-8hg8-63c5-gwmx's check on the raw `options.require === false` input, which only fired for one syntactic shape and missed every other configuration that collapses to the same insecure resolver — see GHSA-m4wx-m65x-ghrr.
18561856
18571857
### Description
18581858
1859-
`NodeVM`'s `nesting: true` option injects a `NESTING_OVERRIDE` builtin that exposes the `vm2` package to sandbox code regardless of any other `require` configuration. The override is unconditional — it survives `require: false`, narrow `builtin` allowlists, and every other restriction the user might set. With `vm2` reachable, the sandbox constructs an inner `NodeVM` whose `require` config is **chosen by the sandbox code, not constrained by the outer config** (this is by design of `nesting`). The inner NodeVM can be configured with `child_process`, `fs`, or any other host module → full host RCE.
1859+
`NodeVM`'s `nesting` option (when truthy) injects a `NESTING_OVERRIDE` builtin that exposes the `vm2` package to sandbox code regardless of any other `require` configuration. The override is unconditional — it survives `require: false`, narrow `builtin` allowlists, and every other restriction the user might set. With `vm2` reachable, the sandbox constructs an inner `NodeVM` whose `require` config is **chosen by the sandbox code, not constrained by the outer config** (this is by design of `nesting`). The inner NodeVM can be configured with `child_process`, `fs`, or any other host module → full host RCE.
18601860
1861-
The trap is **any** `NodeVM` configuration where `nesting: true` is combined with a falsy/omitted `require`. `makeResolverFromLegacyOptions(falsy, NESTING_OVERRIDE, ...)` short-circuits on its `if (!options)` branch and returns a resolver whose only builtin is `vm2` — a pure escape primitive with no legitimate use:
1861+
The trap is **any** `NodeVM` configuration where a truthy `nesting` is combined with a `require` that isn't a real require-config object. Three input-shape classes all collapse to the same NESTING_OVERRIDE-only resolver:
18621862
1863-
- `{ nesting: true, require: false }` ← GHSA-8hg8-63c5-gwmx PoC (explicit deny + nesting)
1864-
- `{ nesting: true }` ← GHSA-m4wx-m65x-ghrr PoC (default require kicks in)
1865-
- `{ nesting: true, require: undefined }` ← destructuring default still applies
1866-
- `{ nesting: true, require: null }``null` is also `!options`-truthy in resolver-compat
1863+
- **Falsy / omitted `require`** — `{ nesting: true }`, `{ nesting: true, require: false / undefined / null / 0 / '' }`. `makeResolverFromLegacyOptions(falsy, NESTING_OVERRIDE, …)` hits its `if (!options)` branch.
1864+
- **Truthy non-object `require`** — `{ nesting: true, require: true / 1 / 'yes' / Symbol() / function(){} }`. `makeResolverFromLegacyOptions` destructures every primitive/function value to all-`undefined`, then calls `makeBuiltinsFromLegacyOptions(undefined, …, NESTING_OVERRIDE)` — the same call shape the `if (!options)` branch produces.
1865+
- **Truthy non-`true` `nesting`** — `{ nesting: 1 / 'yes' / {} / [] / function(){}, require: false }`. The override gate inside the constructor is `nesting && NESTING_OVERRIDE`, which fires for ANY truthy value.
18671866
1868-
All four collapse to the same insecure resolver. The original GHSA-8hg8 patch tested only the first shape with strict equality on the raw input, leaving the other three as bypasses.
1867+
All shapes produce a resolver whose only builtin is `vm2` — a pure escape primitive with no legitimate use. The original GHSA-8hg8 patch tested only the literal `{ nesting: true, require: false }` shape with strict equality on the raw input, leaving every other path as a bypass.
18691868
1870-
CWE-284 (Improper Access Control). CWE-697 (Incorrect Comparison) for the GHSA-m4wx bypass specifically.
1869+
CWE-284 (Improper Access Control). CWE-697 (Incorrect Comparison) for the original GHSA-8hg8 check, which compared too narrowly against the set of values that reach the insecure resolver.
18711870
18721871
### Attack Flow
18731872
@@ -1913,22 +1912,30 @@ The "mental-model mismatch" framing applies at two levels: the *configuration* t
19131912
19141913
### Mitigation
19151914
1916-
`NodeVM` constructor (`lib/nodevm.js`) destructures options first, then throws `VMError` when `nesting === true && !requireOpts`. The check now lives on the value that actually drives `makeResolverFromLegacyOptions`, so every falsy/omitted path collapses to the same rejection regardless of how the embedder wrote the option:
1915+
`NodeVM` constructor (`lib/nodevm.js`) destructures options first, then throws `VMError` when *any truthy `nesting`* is paired with a `requireOpts` that isn't a real require-config object (or a `Resolver` instance). The check lives on the value that actually drives `makeResolverFromLegacyOptions`, so every shape that collapses to the NESTING_OVERRIDE-only resolver collapses to the same rejection:
19171916
19181917
```javascript
19191918
const { require: requireOpts = false, nesting = false, /* ... */ } = options;
1920-
if (nesting === true && !requireOpts) {
1921-
throw new VMError('NodeVM `nesting: true` requires an explicit `require` config. …');
1919+
const hasRealRequireConfig =
1920+
requireOpts instanceof Resolver
1921+
|| (typeof requireOpts === 'object' && requireOpts !== null);
1922+
if (nesting && !hasRealRequireConfig) {
1923+
throw new VMError('NodeVM `nesting` requires an explicit `require` config object. …');
19221924
}
19231925
```
19241926
1925-
This restores **Defense Invariant: Configurations that produce a NESTING_OVERRIDE-only resolver must fail loudly at construction.** The escape hatch (`nesting: true` + an explicit `require` config object, even `{}`) continues to work — the developer's "I accept the trade-off" signal is visible in the call site.
1927+
The guard mirrors the actual reachability of the insecure resolver on two axes:
1928+
1929+
- **`nesting` checked as truthy**, matching the `nesting && NESTING_OVERRIDE` gate that decides whether `vm2` is exposed. Covers `nesting: true / 1 / 'yes' / {} / [] / function(){}` uniformly.
1930+
- **`requireOpts` must be a non-null object** (or a custom `Resolver` instance). Primitives and functions all destructure to all-undefined inside `makeResolverFromLegacyOptions` and produce the insecure resolver, so they are rejected.
1931+
1932+
This establishes **Defense Invariant: Configurations that produce a NESTING_OVERRIDE-only resolver must fail loudly at construction.** The escape hatch (any truthy `nesting` + an explicit `require` config object, even `{}` or `Object.create(null)`) continues to work — the developer's "I accept the trade-off" signal is visible in the call site.
19261933
19271934
### Detection Rules
19281935
1929-
- **`new NodeVM({ nesting: true, ... })`** with any falsy/omitted `require` setting — flagged at construction with `VMError` mentioning GHSA-m4wx-m65x-ghrr.
1936+
- **`new NodeVM({ nesting: <truthy>, ... })`** with `require` set to anything other than a non-null object (or `Resolver`) — flagged at construction with `VMError` mentioning GHSA-m4wx-m65x-ghrr. Covers `require: false / undefined / null / 0 / '' / true / 1 / 'yes' / Symbol() / function(){}` and `nesting` values `true / 1 / 'yes' / {} / [] / function(){}`.
19301937
- **`new NodeVM({ nesting: true })`** with no `require` field at all — closed by GHSA-m4wx-m65x-ghrr (was the loophole the original GHSA-8hg8 fix left open).
1931-
- **Sandbox code containing `require('vm2')`** — only reachable when `nesting: true` *and* an explicit `require` config; almost always indicates an escape attempt unless the embedder explicitly built a VM-spawning host integration.
1938+
- **Sandbox code containing `require('vm2')`** — only reachable when `nesting` is truthy *and* an explicit `require` config object was supplied; almost always indicates an escape attempt unless the embedder explicitly built a VM-spawning host integration.
19321939
19331940
### Considered Attack Surfaces
19341941
@@ -3103,7 +3110,7 @@ The most dangerous attacks combine multiple categories. Each pattern references
31033110
| Array species self-return | set/defineProperty traps + neutralizeArraySpecies + SPECIES_ATTACK_SENTINEL |
31043111
| Host prepareStackTrace fallback | Safe default always set; setter resets to safe default instead of `undefined` |
31053112
| NodeVM `require.root` symlink bypass | `isPathAllowed` realpaths candidate before prefix check; `rootPaths` canonicalized at construction; deny-by-default if realpath throws |
3106-
| NodeVM `nesting: true` + falsy/omitted `require` config trap | Constructor destructures first, then throws `VMError` whenever `nesting === true && !requireOpts` (covers `require: false`, `undefined`, `null`, `0`, and the field being omitted). Citing GHSA-m4wx-m65x-ghrr (supersedes GHSA-8hg8-63c5-gwmx) and the README escape-hatch section |
3113+
| NodeVM `nesting` + non-config `require` trap (NESTING_OVERRIDE-only resolver) | Constructor destructures first, then throws `VMError` whenever `nesting` is truthy and `requireOpts` is not a non-null object or `Resolver`. Covers every value that collapses to the same insecure resolver: falsy `require` (`false`/`undefined`/`null`/`0`/`''`/omitted), truthy non-object `require` (`true`/number/string/symbol/function), and truthy non-true `nesting` (`1`/`'yes'`/`{}`/`[]`/function). Citing GHSA-m4wx-m65x-ghrr (supersedes GHSA-8hg8-63c5-gwmx) and the README escape-hatch section |
31073114
| Sandbox-realm null-proto via bridge `from()` set-trap write-through (GHSA-9vg3-4rfj-wgcm) | `handleException` and sandbox-Promise.then onFulfilled use `ensureThis` (sandbox-realm passthrough); host-Promise rejection sanitiser composes `from()` outside `handleException` so the GHSA-mpf8 invariant still wraps host null-proto values |
31083115
| Internal state probe via computed property access on `globalThis` (GHSA-2cm2-m3w5-gp2f) | Bootstrap script declares `let VM2_INTERNAL_STATE_…` at script-top so the binding lands in the context's `[[GlobalLexicalEnvironment]]`; transformer-emitted `${INTERNAL_STATE_NAME}.handleException(…)` resolves there as before, but `globalThis[k]`, `Reflect.get`, descriptor APIs, and own-property enumeration cannot reach it (the global object's own-key table no longer contains the entry). Supersedes the identifier-only mitigation of GHSA-wp5r-2gw5-m7q7 by closing the entire computed-key class structurally. |
31093116
| Bridge-internal container via `Array.prototype[N]` setter (Category 28: GHSA-9qj6-qjgg-37qq Variant A + GHSA-q3fm-4wcw-g57x Variant B) | Variant A — `neutralizeArraySpeciesBatch` in `lib/bridge.js` writes saved entries via `thisReflectDefineProperty`; appended slot is an own data property and no sandbox-installed setter is invoked while the bridge holds raw saved state. Variant B — `defaultSandboxPrepareStackTrace` in `lib/setup-sandbox.js` accumulates frames in a string via primitive concatenation rather than an array, removing every reachable `Array.prototype` slot (index setter, getter, and `.join`); `makeCallSiteGetters` installs entries via `localReflectDefineProperty` for symmetry |

0 commit comments

Comments
 (0)