Skip to content

Commit e1c48fc

Browse files
committed
fix(GHSA-9g8x-92q2-p28f): deny process-wide observability builtins in NodeVM
NodeVM's `require.builtin` allowlist defended against host-code-loading primitives (`module`, `worker_threads`, `cluster`, `vm`, `repl`, `inspector`, `process`, `trace_events`, `wasi`) but did not cover a parallel class of builtins whose primary capability is reading state of the entire host Node process. Each gave a one-line exfiltration primitive once the sandbox held a readonly proxy over the host module: - `require('diagnostics_channel').channel('http.server.request.start') .subscribe(cb)` — sandbox callback receives raw host `IncomingMessage` objects for every HTTP request the embedder serves, with full `Authorization`, `Cookie`, `x-session-token` headers intact. - `require('async_hooks').executionAsyncResource()` — returns the current host `AsyncResource`. Embedders using `AsyncLocalStorage` for per-request user/auth context (`express`, `fastify`, `next.js` patterns) pin that state on the resource; the sandbox reads it directly. - `require('perf_hooks').performance.getEntriesByType('mark')` — reads every host-side `performance.mark(name)`. Production code routinely embeds request IDs, user IDs, route paths, or partial query strings in mark names. - `require('v8').getHeapSnapshot()` / `.writeHeapSnapshot(path)` / `.queryObjects(Ctor)` — full host V8 heap serialization (every string, every Buffer, every closure capture) to memory or arbitrary host filesystem path; Node 20+ `queryObjects` enumerates every host-realm instance of a constructor. Not RCE — the underscored or named modules load through the normal `vm.readonly()` wrap. The impact is information disclosure: the data the embedder routes through these APIs in the same process *is* host data, by spec. Wrapping the module in a proxy cannot localize the data source. Structural fix in `lib/builtin.js`: - `DANGEROUS_BUILTINS` extended with `diagnostics_channel`, `async_hooks`, `perf_hooks`, `v8`. Reuses the existing four-layer enforcement established by GHSA-947f-4v7f-x2v8 and tightened by GHSA-rp36-8xq3-r6c4 (family-prefix + `node:` normalization via `isDangerousBuiltin`). - `v8` was added during the fix beyond the originally-named three — `v8.writeHeapSnapshot(path)` is strictly worse than `perf_hooks` for the same invariant, so excluding it would leave a wide same-class bypass. - `mocks` / `overrides` / `SPECIAL_MODULES` escape hatches preserved: embedders who genuinely need a sandbox-local clock or async context can register a controlled wrapper under the same name. The denylist only rejects the default host-passthrough loader. Reinforces Defense Invariant #13 (NodeVM builtin allowlist is a closed system) — extending its scope from code-execution primitives to information-disclosure surfaces. Tests: `test/ghsa/GHSA-9g8x-92q2-p28f/repro.js` (251 lines) covers all four builtins, both bare-name and `node:`-prefixed spellings, the wildcard expansion path, the explicit-allowlist path, the low-level `makeBuiltins` API, and confirms `mocks` / `overrides` escape hatches still work. Docs: `docs/ATTACKS.md` gains Category 35 (renumbered from the branch's provisional Category 30 — that slot is now occupied by GHSA-v6mx host prototype mutation; Cats 31-34 are also already taken). Category 21's DANGEROUS_BUILTINS list cross-references Category 35 for the four added names. New row in the "How The Bridge Defends" table. Version bump deferred — this lands alongside the other advisories in the pre-existing [3.11.4] release slot.
1 parent 436053e commit e1c48fc

4 files changed

Lines changed: 383 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## [3.11.4]
44

5-
Nine advisories closed. Patch release — no API changes for valid configurations.
5+
Ten advisories closed. Patch release — no API changes for valid configurations.
66

77
### Security fixes
88

@@ -15,6 +15,7 @@ Nine advisories closed. Patch release — no API changes for valid configuration
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/`.
18+
- **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/`.
1819

1920
### Upgrade notes
2021

docs/ATTACKS.md

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,7 +1607,7 @@ The user's mental model of `['*', '-child_process']` is "every builtin except `c
16071607
16081608
Three-layer denylist enforcement in `lib/builtin.js` (restores **[Invariant 13 — The NodeVM builtin allowlist is a closed system](#defense-invariants)**):
16091609
1610-
1. **`DANGEROUS_BUILTINS` Set** at module load — `['module', 'worker_threads', 'cluster', 'vm', 'repl', 'inspector', 'process', 'trace_events', 'wasi']`.
1610+
1. **`DANGEROUS_BUILTINS` Set** at module load — `['module', 'worker_threads', 'cluster', 'vm', 'repl', 'inspector', 'process', 'trace_events', 'wasi', 'diagnostics_channel', 'async_hooks', 'perf_hooks', 'v8']`. The last four were added by [Category 35](#attack-category-35-nodevm-process-wide-observability-builtins-host-data-info-leak) for the process-wide observability info-leak class; they share the deny-by-default enforcement but a different threat model (data exposure, not code execution).
16111611
2. **Family-prefix check** via `isDangerousBuiltin(key)` — any `<family>/...` whose family is in the denylist is also blocked (e.g. `inspector/promises`, future `inspector/foo`, hypothetical `process/foo`, `module/foo`). The check also strips the optional `node:` URL-style prefix so `node:process` and `node:inspector/promises` are caught.
16121612
3. **Filter from `BUILTIN_MODULES`** — closes the `'*'` wildcard expansion path. `'*'` will never auto-allow these names regardless of the user's exclusion list.
16131613
4. **Reject in `addDefaultBuiltin`** — closes the explicit-allowlist path (`builtin: ['module']`, `builtin: ['process']`, `builtin: ['inspector/promises']`) and the lower-level `makeBuiltins([...])` API used by custom resolvers. The `SPECIAL_MODULES` escape hatch is preserved: a future safe wrapper (e.g. a `module` shim that exposes only `builtinModules` metadata) can be registered there if a real consumer needs it.
@@ -1637,7 +1637,7 @@ The fix does not affect the `mocks` / `overrides` escape hatches — users who g
16371637
16381638
### Considered Attack Surfaces
16391639
1640-
- **`async_hooks`** exposes context tracing but not host-code-loading primitives. Allowed under `'*'`.
1640+
- **`async_hooks`, `diagnostics_channel`, `perf_hooks`, `v8`** are now denied as process-wide observability primitives — see [Category 30](#attack-category-30-nodevm-process-wide-observability-builtins-host-data-info-leak). They expose host-process state rather than host-code-loading primitives, but are functionally identical from the embedder's perspective: any allowlist that includes them leaks per-request user data, auth tokens, and heap contents into the sandbox.
16411641
- **`child_process`** is NOT on the auto-denylist because users may legitimately want it for trusted scripts (e.g., dev tooling running known scripts in vm2 for hot-reload isolation). For untrusted code, `child_process` is a full-host-RCE primitive — embedders MUST exclude it explicitly (`['*', '-child_process']`) or, better, use an explicit allowlist of just the modules they need. The README's "Hardening recommendations" section calls this out.
16421642
- **`fs`** is allowed under `'*'` because file-system access can be a legitimate sandbox capability for many use cases (e.g., user-script template engines reading templates). Users who want filesystem isolation use `VMFileSystem` or exclude `fs` explicitly. Same caveat as `child_process``'*'` is not sandbox-safe for untrusted code.
16431643
- **`dgram`, `net`, `http`, `https`, `dns`** are network-IO builtins, allowed under `'*'`. Any of them give untrusted code outbound network access from the host. Embedders should explicitly exclude or allowlist.
@@ -2891,6 +2891,96 @@ None. This fix complements [Category 21](#attack-category-21-nodevm-builtin-allo
28912891
28922892
---
28932893
2894+
## Attack Category 35: NodeVM Process-Wide Observability Builtins (Host-Data Info Leak)
2895+
2896+
### Description
2897+
2898+
NodeVM's `require.builtin` allowlist defends sandbox code from reaching dangerous Node modules. [Category 21](#attack-category-21-nodevm-builtin-allowlist-bypass-via-host-passthrough-builtins) denied the host-code-loading primitives (`module`, `worker_threads`, `cluster`, `vm`, `repl`, `inspector`, `process`, `trace_events`, `wasi`). A second class of dangerous builtins exists with a different threat model: **process-wide observability modules** whose primary capability is reading state of the entire host Node process, not loading or executing code.
2899+
2900+
When such a builtin is reachable from the sandbox (via the `'*'` wildcard or an explicit allowlist), the sandbox can subscribe to or read host process state directly — no RCE chain needed. The data the embedder routes through these APIs in the same process (HTTP requests, async-context user IDs, performance marks, V8 heap) is by definition host data; reaching the host module *is* the escape.
2901+
2902+
CWE-668 (Exposure of Resource to Wrong Sphere). Info-leak class, not RCE class.
2903+
2904+
### Attack Flow
2905+
2906+
Each builtin gives a one-liner exfiltration primitive. Once the sandbox holds a readonly proxy over the host module, the proxy's `apply` trap forwards every method call back to the host realm:
2907+
2908+
- **`diagnostics_channel`** — `dc.channel('http.server.request.start').subscribe(cb)`. The sandbox callback receives raw host `IncomingMessage` objects for every HTTP request the embedder serves, with full `Authorization`, `Cookie`, `x-session-token` headers intact.
2909+
- **`async_hooks`** — `async_hooks.executionAsyncResource()` returns the current host `AsyncResource`. Embedders that use `AsyncLocalStorage` for per-request user/auth context (extremely common pattern: `express`, `fastify`, `next.js`) pin that state on the resource, and the sandbox reads it directly.
2910+
- **`perf_hooks`** — `perf_hooks.performance.getEntriesByType('mark')` reads every host-side `performance.mark(name)`. Production code routinely embeds request IDs, user IDs, route paths, or partial query strings into mark names for observability dashboards.
2911+
- **`v8`** — `v8.getHeapSnapshot()` returns a Readable stream of the entire host V8 heap (every string, every Buffer, every closure capture). `v8.writeHeapSnapshot(path)` writes the same to an arbitrary host filesystem path. `v8.queryObjects(Ctor)` (Node 20+) returns every host-realm instance of a constructor.
2912+
2913+
### Canonical Example
2914+
2915+
```javascript
2916+
// (advisory GHSA-9g8x-92q2-p28f)
2917+
const vm = new NodeVM({ require: { builtin: ['*'], external: false } });
2918+
vm.run(`
2919+
const dc = require('diagnostics_channel');
2920+
const stolen = [];
2921+
dc.channel('http.server.request.start').subscribe((req) => {
2922+
stolen.push({
2923+
url: req.url,
2924+
authorization: req.headers.authorization,
2925+
session: req.headers['x-session-token'],
2926+
});
2927+
});
2928+
// ... wait for host HTTP traffic. Headers are read from inside the sandbox.
2929+
`, 'poc.js');
2930+
```
2931+
2932+
Equivalent one-liners for the other three:
2933+
2934+
```javascript
2935+
require('async_hooks').executionAsyncResource(); // -> host AsyncResource
2936+
require('perf_hooks').performance.getEntriesByType('mark'); // -> host marks
2937+
require('v8').writeHeapSnapshot('/tmp/host-heap.json'); // -> entire host heap on disk
2938+
```
2939+
2940+
### Why It Works
2941+
2942+
The vm2 boundary is built around the assumption that "the sandbox observes its own realm, not the host's". Most Node builtins satisfy this implicitly: `path.join(...)`, `crypto.randomBytes(...)`, `url.parse(...)` all operate on inputs the sandbox passes in and return values the sandbox owns. The bridge's `ReadOnlyHandler` makes those builtins safe via uniform proxy semantics.
2943+
2944+
Process-wide observability builtins break the assumption because the data they surface *is* host data by spec — `executionAsyncResource()` returns "the resource currently executing" measured against the host's call stack, not the sandbox's. Wrapping the module in a proxy does not localize the data source. The bridge cannot usefully sanitize the values because they're real host objects (IncomingMessage, AsyncResource), and stripping them to primitives would defeat the embedder's reason for ever exposing the module in the first place.
2945+
2946+
The four builtins in scope all share this property: they observe a process resource (HTTP request hook, async context, perf timeline, V8 heap). Mitigation must therefore be "deny by default", not "proxy more carefully".
2947+
2948+
### Mitigation
2949+
2950+
Extend `DANGEROUS_BUILTINS` in `lib/builtin.js` with the four observability names. Reuses the same enforcement established by Category 21 (now four-layer after the `isDangerousBuiltin` family-prefix promotion):
2951+
2952+
1. **Filtered out of `BUILTIN_MODULES`** — closes the `'*'` wildcard expansion path. `builtin: ['*']` and `builtin: ['*', '-fs']` no longer auto-allow these names.
2953+
2. **Rejected in `addDefaultBuiltin`** via `isDangerousBuiltin(key)` — closes the explicit-allowlist path (`builtin: ['perf_hooks']`), the object-map form (`builtin: { v8: true }`), and the lower-level `makeBuiltins(['async_hooks'])` API used by custom resolvers.
2954+
3. **Family-prefix check** — any `<family>/...` whose family is in the denylist is also blocked (e.g. hypothetical `perf_hooks/foo`).
2955+
4. **`node:` prefix stripped before lookup** — `require('node:diagnostics_channel')` resolves identically to the bare name and is blocked by the same denial.
2956+
2957+
The `SPECIAL_MODULES`, `mocks`, and `overrides` escape hatches are preserved: an embedder who genuinely needs sandbox-local timing or async context can register a controlled wrapper under the same name (e.g., a `perf_hooks` shim that only exposes a sandbox-local clock). The denylist only rejects the *default host-passthrough loader*.
2958+
2959+
`v8` was added during this fix beyond the originally-named three. The class is "process-wide observability modules"; `v8.writeHeapSnapshot(path)` is strictly worse than `perf_hooks` against the same invariant (writes a full heap dump to an arbitrary host filesystem path), so excluding it would leave a wide bypass of the same class.
2960+
2961+
The fix restores **[Defense Invariant #13](#defense-invariants)** at a different layer — the NodeVM builtin allowlist is a closed system, regardless of whether the threat is code execution or data exposure. The bridge invariant still holds for these modules; the deny-list ensures the bridge is never asked to wrap them in the first place.
2962+
2963+
### Detection Rules
2964+
2965+
- **`builtin: ['*']` or `builtin: ['*', '-X']`** in NodeVM config — historically auto-allowed `diagnostics_channel`, `async_hooks`, `perf_hooks`, `v8`. Now filtered. Same caveat as Category 21: `'*'` still allows `fs`, `child_process` (if not excluded), `net`, `http`, `dns` — not a sandbox-safe default for untrusted code.
2966+
- **`require('diagnostics_channel').channel(...).subscribe(...)`** — host HTTP/DB/IPC observability subscription.
2967+
- **`require('async_hooks').executionAsyncResource()` / `.createHook({...}).enable()`** — host async context inspection.
2968+
- **`require('perf_hooks').performance.getEntriesByType('mark' | 'measure' | 'resource')`** — host performance timeline read.
2969+
- **`require('v8').getHeapSnapshot()` / `.writeHeapSnapshot(path)`** — full host heap exfiltration to memory or disk.
2970+
- **`require('v8').queryObjects(Ctor)`** (Node 20+) — enumeration of host-realm instances of a constructor.
2971+
- **Sandbox code that subscribes to channels named `http.server.request.*`, `http.client.request.*`, `dns.lookup.*`, `net.client.socket.*`** — these are the canonical diagnostic channels used by Node core and request-tracking libraries.
2972+
2973+
### Considered Attack Surfaces
2974+
2975+
- **`os`** exposes hostname, network interfaces, user info. The data is host environment, not per-request, and is generally considered configuration metadata rather than tenant data. Allowed under `'*'` for consistency with `process.env` exposure expectations. Embedders who consider hostname/`userInfo()` sensitive should exclude `os` explicitly.
2976+
- **`dns`** can resolve internal hostnames and exfiltrate via DNS lookup. Network-IO class, same as `http`/`net`. Not in this denylist — embedders who care about network isolation must allowlist explicitly. Documented in Category 21's "Considered Attack Surfaces".
2977+
- **`zlib`, `crypto`, `string_decoder`, `buffer`** — sandbox-local data transforms, no host-state observability. Safe under default proxy semantics.
2978+
- **`process`** — already denied via Category 21 (after GHSA-rp36-8xq3-r6c4 extended `DANGEROUS_BUILTINS`). The sandbox global `process` is a curated stub defined in `lib/setup-node-sandbox.js`.
2979+
- **`worker_threads.parentPort` and `worker_threads.workerData`** — would expose host worker IPC channel and initial data. Already denied by Category 21 (entire `worker_threads` module is denied; this category is a different threat model on top, not a subset).
2980+
- **`http`, `https`, `http2`, `net`, `tls`, `dgram`** — network-IO modules. These do *not* observe existing host state; they originate new connections. Different threat model (outbound network from host) — covered in Category 21's "Considered Attack Surfaces" and Category 34 (underscored siblings). Embedders who want network isolation must exclude or replace them.
2981+
2982+
---
2983+
28942984
## Considered Attack Surfaces
28952985
28962986
These attack surfaces were analyzed and found to be safe or low-risk. They are documented here so future reviewers do not re-investigate them.
@@ -3021,6 +3111,7 @@ The most dangerous attacks combine multiple categories. Each pattern references
30213111
| Bridge `set` trap ignores spec `Receiver` (GHSA-c4cf-2hgv-2qv6) | `BaseHandler.set` gates host-write forwarding on `receiver === mappingOtherToThis.get(object)`; non-canonical receivers (inherited-receiver writes via `Object.create(proxy)`, forged-receiver `Reflect.set` calls, `Object.assign(child, src)` loops) install on `receiver` via `Reflect.defineProperty`, mirroring `ReadOnlyHandler.set` |
30223112
| NodeVM builtin denylist bypass via `process` / `inspector/promises` (GHSA-rp36-8xq3-r6c4) | `DANGEROUS_BUILTINS` extended to include `process`; matching promoted to family-prefix via `isDangerousBuiltin(key)` so subpath builtins (`inspector/promises`, future `inspector/*`, `process/*`, `module/*`) share fate with their canonical name. `node:` URL prefix stripped before lookup. Enforced at both `BUILTIN_MODULES` source and `addDefaultBuiltin`. Supersedes the GHSA-947f-4v7f-x2v8 exact-match mitigation. |
30233113
| NodeVM wildcard exposes underscored network builtins (GHSA-r9pm-gxmw-wv6p) | `BUILTIN_MODULES` filter in `lib/builtin.js` now excludes any name starting with `_`; `'*'` no longer expands to `_http_client`/`_http_server`/`_tls_wrap`/`_stream_*` etc. Explicit opt-in (`builtin: ['_http_client']`) and `mock`/`override` paths still work via `addDefaultBuiltin`. |
3114+
| NodeVM process-wide observability builtins (GHSA-9g8x-92q2-p28f) | `DANGEROUS_BUILTINS` denylist extended with `diagnostics_channel`, `async_hooks`, `perf_hooks`, `v8`; filtered out of `BUILTIN_MODULES` (closes `'*'` wildcard) and rejected in `addDefaultBuiltin` via `isDangerousBuiltin` (closes explicit allowlist and `makeBuiltins([...])`). `node:` prefix normalized and family-prefix subpath matching applied. `mocks`/`overrides` escape hatch preserved for sandbox-local replacements |
30243115
30253116
### Key Security Invariant: Promise Species Resolution Timing
30263117

0 commit comments

Comments
 (0)