Added punycode to native modules & option to run without 'use strict;'#13
Merged
Merged
Conversation
patriksimek
added a commit
that referenced
this pull request
Dec 11, 2015
Added punycode to native modules & option to run without 'use strict;'
Owner
|
Thanks. |
patriksimek
added a commit
that referenced
this pull request
May 18, 2026
…ocess/inspector subpaths
The denylist in `lib/builtin.js` was exact-match, so two host-passthrough
families slipped through every enforcement layer:
- `require('process').getBuiltinModule(name)` (Node 22+) reloads ANY core
module — including `child_process` — regardless of the embedder's
allow/deny configuration. Also exposes `binding`, `dlopen`,
`_linkedBinding`, and the raw host `process.env`.
- `require('inspector/promises').Session().post('Runtime.evaluate', ...)`
evaluates attacker JS in the host realm. The subpath shape
`inspector/promises` does not match the `inspector` denylist entry.
Both reached host RCE under the otherwise-correct configs
`builtin: ['*', '-child_process']` and `builtin: ['*', '-child_process',
'-inspector']`.
Structural fix in `lib/builtin.js`:
- `DANGEROUS_BUILTINS` extended with `process`.
- New `isDangerousBuiltin(key)` promotes the check to family-prefix
matching: any `<family>/...` whose family is in the set is blocked. The
`node:` URL-style prefix is stripped before matching so
`node:process` and `node:inspector/promises` share fate with their
canonical spellings.
- Enforced at both `BUILTIN_MODULES` (closes the `'*'` wildcard expansion
path) and `addDefaultBuiltin` (closes the explicit-allowlist path and
the low-level `makeBuiltins` API).
- `SPECIAL_MODULES` / `mocks` / `overrides` escape hatches preserved —
embedders who genuinely need a stub for any blocked name can register a
sandbox-safe replacement.
Supersedes GHSA-947f-4v7f-x2v8 (whose exact-match denylist missed both
families). Restores and tightens Defense Invariant #13 (NodeVM builtin
allowlist is a closed system) — added in this commit.
Tests: `test/ghsa/GHSA-rp36-8xq3-r6c4/repro.js` (175 lines) covers
`require('process').getBuiltinModule('child_process')`,
`require('inspector/promises').Session().post('Runtime.evaluate', ...)`,
the `node:` URL-style spellings, the explicit-allowlist path
(`builtin: ['process']` and `builtin: ['inspector/promises']`), the
low-level `makeBuiltins` API, and confirms `SPECIAL_MODULES` overrides
still work.
Docs: `docs/ATTACKS.md` Category 21 extended with the new attack flow,
both canonical PoCs, and the three-layer mitigation. New Defense
Invariant #13 added under the existing #12 (WebAssembly JSPI). New row
in the "How The Bridge Defends" table.
Version bump deferred — this lands alongside seven other advisories in
the pre-existing [3.11.4] release slot.
patriksimek
added a commit
that referenced
this pull request
May 18, 2026
…' wildcard expansion
NodeVM's `'*'` wildcard expansion in `lib/builtin.js` sourced the allowed
builtin list from `require('module').builtinModules`, filtered only against
`internal/*` and `DANGEROUS_BUILTINS`. Node's private implementation siblings
of network and stream modules — `_http_client`, `_http_server`, `_http_agent`,
`_http_common`, `_http_incoming`, `_http_outgoing`, `_tls_common`, `_tls_wrap`,
and the `_stream_*` family — passed both filters, so they were silently
admitted under the documented `builtin: ['*', '-http', '-https', '-net',
'-dgram', '-tls', '-dns', '-http2']` exclusion pattern.
This handed the sandbox the network primitives the embedder had explicitly
attempted to remove:
- `require('_http_client').ClientRequest({host, port, path})` — outbound
HTTP request, bypasses `-http`/`-https`.
- `require('_http_server').Server(handler).listen(0)` — listening HTTP
socket, bypasses `-net`.
- `require('_tls_wrap').TLSSocket` — TLS primitives, bypasses `-tls`.
Not RCE — the underscored modules load through the normal `vm.readonly()`
wrap. The impact is capability bypass: SSRF-class access to localhost
services, cloud metadata endpoints, internal admin panels, and any other
network resource the host process can reach (CVSS 8.6).
Structural fix in `lib/builtin.js`:
- `BUILTIN_MODULES` filter extended with `!s.startsWith('_')`. The `'*'`
wildcard now expands only to documented public Node builtins.
- Forward-compatible: any new underscored builtin Node introduces in a
future release is automatically excluded without requiring a vm2
release.
- `addDefaultBuiltin` is unchanged, so explicit opt-in
(`builtin: ['_http_client']`, `makeBuiltins(['_http_client'])`),
`mocks`, and `overrides` registrations under underscored names continue
to work — embedders who genuinely need an underscored sibling can still
name it.
Complements the GHSA-rp36-8xq3-r6c4 fix that just landed (Category 21 /
Defense Invariant #13: host-passthrough primitives are unreachable under
any config). Together they enforce: "no host-passthrough primitive AND
no undocumented underscored sibling is reachable under `builtin: ['*']`."
Tests: `test/ghsa/GHSA-r9pm-gxmw-wv6p/repro.js` (202 lines) covers
`require('_http_client')`, `require('_http_server')`, `require('_tls_wrap')`,
the `node:`-prefixed spellings, the `_stream_*` family, the canonical
`ClientRequest` unreachability check, and confirms the explicit-opt-in /
`mock` / `override` escape hatches still function.
Docs: `docs/ATTACKS.md` gains Category 34 (renumbered from the branch's
provisional Category 30 — that slot is now occupied by GHSA-v6mx host
prototype mutation). 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.
patriksimek
added a commit
that referenced
this pull request
May 18, 2026
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added punycode to native modules & option to run without 'use strict;'