Skip to content

Commit 768bcfc

Browse files
fix(GHSA-m5w8-4gq2-6f8x): deny os and dns NodeVM builtins
Root cause: `os` and `dns` were the two remaining members of the process-wide builtin class closed by GHSA-9g8x-92q2-p28f. Under `builtin: ['*']` they loaded through the default `vm.readonly(hostRequire(key))` path, which cannot localise host-process state -- and both carry write APIs (`dns.setServers`, `dns.setDefaultResultOrder`, `os.setPriority`) that mutate global host state from one line of sandbox code. Fix: add 'os' and 'dns' to DANGEROUS_BUILTINS in lib/builtin.js. The existing family-prefix matcher `isDangerousBuiltin` extends this to `node:os`, `node:dns`, and `dns/promises` with no further change, and both enforcement layers (the `BUILTIN_MODULES` wildcard filter and `addDefaultBuiltin` rejection) apply automatically. Restores Defense Invariant #13 -- the NodeVM builtin allowlist is a closed system. The `mock` / `override` escape hatch is preserved for embedders needing a sandbox-local subset. Tests: test/ghsa/GHSA-m5w8-4gq2-6f8x/repro.js. Also updates test/ghsa/GHSA-rp36-8xq3-r6c4/repro.js, whose "safe siblings still load" case used `dns/promises` as an example of a permitted subpath; it now uses `fs/promises` and `stream/promises`, since `dns/promises` is denied by this change. docs/ATTACKS.md: Category 35 extended with the os/dns read and write primitives. No version bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5e0f255 commit 768bcfc

5 files changed

Lines changed: 375 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ Ten advisories closed. Patch release — no API changes for valid configurations
2525
- **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/`.
2626
- **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/`.
2727
- **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/`.
28+
- **GHSA-m5w8-4gq2-6f8x** — sibling of GHSA-9g8x: NodeVM `builtin: ['*']` still surfaced `os` and `dns`, the two remaining process-wide builtins. Beyond host-identity and network-topology reads, both carry *write* APIs reachable in one line of sandbox code — `dns.setServers()` hijacks the host's process-wide DNS resolver and `os.setPriority()` renices the host process. Fix in `lib/builtin.js` extends `DANGEROUS_BUILTINS` with `os` and `dns`, reusing the existing two-layer enforcement; `isDangerousBuiltin` covers `node:os`, `node:dns`, and `dns/promises` automatically, and `mock`/`override` escape hatches are preserved. See ATTACKS.md Category 35 (extended) and `test/ghsa/GHSA-m5w8-4gq2-6f8x/`.
2829

2930
### Upgrade notes
3031

docs/ATTACKS.md

Lines changed: 27 additions & 4 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', '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).
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', 'os', 'dns']`. The last six 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 (`os` and `dns` via GHSA-m5w8-4gq2-6f8x, which additionally close the host-process *write* APIs `os.setPriority` / `dns.setServers` / `dns.setDefaultResultOrder`); they share the deny-by-default enforcement but a different threat model (data exposure / host-state mutation, 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.
@@ -2908,6 +2908,8 @@ When such a builtin is reachable from the sandbox (via the `'*'` wildcard or an
29082908
29092909
CWE-668 (Exposure of Resource to Wrong Sphere). Info-leak class, not RCE class.
29102910
2911+
**Extended by GHSA-m5w8-4gq2-6f8x (`os`, `dns`)** — the same class contains two more builtins whose state belongs to the host process and which additionally expose *process-wide write* APIs. These are strictly worse than the read-only four above: `os.setPriority()` renices the host process, and `dns.setServers()` / `dns.setDefaultResultOrder()` mutate the host's process-wide DNS resolution from one synchronous line of sandbox code. CWE-200 + CWE-732 + CWE-285.
2912+
29112913
### Attack Flow
29122914
29132915
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:
@@ -2916,6 +2918,8 @@ Each builtin gives a one-liner exfiltration primitive. Once the sandbox holds a
29162918
- **`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.
29172919
- **`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.
29182920
- **`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.
2921+
- **`os`** (GHSA-m5w8-4gq2-6f8x) — reads: `os.userInfo()` returns the host process owner (uid/gid/username/homedir/shell); `os.networkInterfaces()` returns the host's full network topology (container/VM veth pairs, IPs, MACs); `os.hostname()` / `os.loadavg()` / `os.uptime()` / `os.freemem()` are host-wide telemetry. Write: `os.setPriority([pid,] prio)` invokes `setpriority(2)` on the host process (pid 0 = host), persisting after the sandbox call returns.
2922+
- **`dns`** (GHSA-m5w8-4gq2-6f8x) — the strongest primitive in the class: `dns.setServers(['attacker:53'])` replaces the host's process-wide DNS resolver list, so every subsequent lookup the host makes (its own outbound HTTP, telemetry, npm registry, `fetch`, URL-based fs paths) flows through the attacker's resolver — a one-line DNS hijack with no rate limit, audit trail, or embedder notification. `dns.setDefaultResultOrder()` is a second process-wide write knob; `dns.getServers()` / `dns.lookup()` / `dns.resolve()` read and act from the host network identity. The `dns/promises` subpath shares the surface and is covered by the same denial via the family-prefix matcher.
29192923
29202924
### Canonical Example
29212925
@@ -2944,17 +2948,34 @@ require('perf_hooks').performance.getEntriesByType('mark'); // -> host marks
29442948
require('v8').writeHeapSnapshot('/tmp/host-heap.json'); // -> entire host heap on disk
29452949
```
29462950
2951+
The `os` / `dns` extension (GHSA-m5w8-4gq2-6f8x) — note the two host-process *writes*:
2952+
2953+
```javascript
2954+
// (advisory GHSA-m5w8-4gq2-6f8x)
2955+
const vm = new NodeVM({ require: { builtin: ['*'], external: false } });
2956+
vm.run(`
2957+
const os = require('os');
2958+
os.userInfo(); // -> host uid/gid/username/homedir/shell
2959+
os.networkInterfaces(); // -> host network topology (IPs, MACs)
2960+
os.setPriority(10); // WRITE: renices the host process
2961+
2962+
require('dns').setServers(['127.0.0.1:5353']); // WRITE: hijacks every
2963+
// subsequent host DNS lookup -- outbound HTTP, telemetry, registry fetch.
2964+
`, 'poc.js');
2965+
// Both writes are observed from the host realm after vm.run() returns.
2966+
```
2967+
29472968
### Why It Works
29482969
29492970
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.
29502971
29512972
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.
29522973
2953-
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".
2974+
The builtins in scope all share this property: they observe a process resource (HTTP request hook, async context, perf timeline, V8 heap) or — for `os` / `dns` — read host-kernel/host-process state and, worse, *write* it. `os.setPriority()` and `dns.setServers()` are not observations at all; they mutate global host-process state, so even a perfect read-side proxy is irrelevant. Mitigation must therefore be "deny by default", not "proxy more carefully".
29542975
29552976
### Mitigation
29562977
2957-
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):
2978+
Extend `DANGEROUS_BUILTINS` in `lib/builtin.js` with the observability names — the original four (`diagnostics_channel`, `async_hooks`, `perf_hooks`, `v8`) plus `os` and `dns` (GHSA-m5w8-4gq2-6f8x). Reuses the same enforcement established by Category 21 (now four-layer after the `isDangerousBuiltin` family-prefix promotion):
29582979
29592980
1. **Filtered out of `BUILTIN_MODULES`** — closes the `'*'` wildcard expansion path. `builtin: ['*']` and `builtin: ['*', '-fs']` no longer auto-allow these names.
29602981
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.
@@ -2965,6 +2986,8 @@ The `SPECIAL_MODULES`, `mocks`, and `overrides` escape hatches are preserved: an
29652986
29662987
`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.
29672988
2989+
`os` and `dns` (GHSA-m5w8-4gq2-6f8x) were the two remaining members of the class. They satisfy the same description (host-process state the `vm.readonly()` proxy cannot localise) and additionally expose *write* primitives — `dns.setServers()` is a one-line process-wide DNS hijack, strictly worse than every read-only leak the original fix closed. Adding the two family names automatically covers `node:os`, `node:dns`, and the `dns/promises` subpath via `isDangerousBuiltin`'s `node:`-strip and family-prefix matching, with no per-API enumeration needed for future Node releases. Embedders who genuinely need a sandbox-local subset (`os.platform()`, `os.EOL`, `os.constants`) register a controlled wrapper via `mock` / `override`, exactly as for the original four.
2990+
29682991
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.
29692992
29702993
### Detection Rules
@@ -3120,7 +3143,7 @@ The most dangerous attacks combine multiple categories. Each pattern references
31203143
| 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` |
31213144
| 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. |
31223145
| 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`. |
3123-
| 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 |
3146+
| NodeVM process-wide observability builtins (GHSA-9g8x-92q2-p28f, GHSA-m5w8-4gq2-6f8x) | `DANGEROUS_BUILTINS` denylist extended with `diagnostics_channel`, `async_hooks`, `perf_hooks`, `v8` and (GHSA-m5w8-4gq2-6f8x) `os`, `dns`; 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 (covers `node:os`, `node:dns`, `dns/promises`). `os.setPriority` / `dns.setServers` / `dns.setDefaultResultOrder` host-process writes closed alongside the read leaks. `mocks`/`overrides` escape hatch preserved for sandbox-local replacements |
31243147
31253148
### Key Security Invariant: Promise Species Resolution Timing
31263149

lib/builtin.js

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,34 @@ const DANGEROUS_BUILTINS = new Set([
135135
'diagnostics_channel',
136136
'async_hooks',
137137
'perf_hooks',
138-
'v8'
138+
'v8',
139+
// SECURITY (GHSA-m5w8-4gq2-6f8x): Same process-wide class as the GHSA-9g8x
140+
// four above, extended with the two builtins that also expose host-process
141+
// state the `vm.readonly()` proxy cannot localise -- and, worse, carry
142+
// *write* APIs that mutate global host state from one line of sandbox code:
143+
//
144+
// - os : `os.userInfo()` leaks the host process owner (uid/gid/username/
145+
// homedir/shell); `os.networkInterfaces()` leaks the full host
146+
// network topology (container/VM veth pairs, IPs, MACs);
147+
// `os.hostname()` / `os.loadavg()` / `os.uptime()` / `os.freemem()`
148+
// are host-wide telemetry. `os.setPriority([pid,] prio)` is a
149+
// *write* -- `setpriority(2)` on the host process (pid 0 = host),
150+
// strictly worse than the read-only v8/perf_hooks family.
151+
// - dns : `dns.setServers(['attacker:53'])` replaces the host's
152+
// process-wide DNS resolver list, hijacking every subsequent
153+
// lookup the host makes (outbound HTTP, telemetry, npm registry,
154+
// fetch, URL-based fs paths) -- a one-line DNS-hijack primitive
155+
// with no rate limit, audit trail, or embedder notification.
156+
// `dns.setDefaultResultOrder()` is a second process-wide write
157+
// knob. `dns.getServers()` / `dns.lookup()` / `dns.resolve()`
158+
// read and act from the host network identity. Covers the
159+
// `dns/promises` subpath via the family-prefix matcher below.
160+
//
161+
// Embedders who genuinely need a sandbox-local subset (typically
162+
// `os.platform()`, `os.EOL`, `os.constants`) can register a controlled
163+
// wrapper under the same name via `mock` / `override`.
164+
'os',
165+
'dns'
139166
]);
140167

141168
// SECURITY (GHSA-rp36-8xq3-r6c4): Family-prefix denylist check. `inspector` and

0 commit comments

Comments
 (0)