Skip to content

prevent RealFSProvider sandbox escape via symlinks#64409

Open
VinzSpring wants to merge 1 commit into
nodejs:mainfrom
VinzSpring:vfs-symlink-sandbox-escape-fix
Open

prevent RealFSProvider sandbox escape via symlinks#64409
VinzSpring wants to merge 1 commit into
nodejs:mainfrom
VinzSpring:vfs-symlink-sandbox-escape-fix

Conversation

@VinzSpring

Copy link
Copy Markdown

Summary

Fix a sandbox escape vulnerability in RealFSProvider where symlinks pointing outside the root directory were silently followed instead of being rejected.

Root Cause

The containment check (throw createENOENT) was performed inside the try { fs.realpathSync() } block in #resolvePath and #verifyAncestorInRoot. Because the thrown error carried an ENOENT code, the adjacent catch handler β€” designed to handle genuinely non-existent paths β€” swallowed the rejection. This caused the raw (unresolved, symlink-containing) path to be returned, allowing callers to follow the symlink out of the sandbox.

Changes

  1. Canonicalize the root at construction time β€” #realRoot is resolved via fs.realpathSync() so that all containment comparisons use the canonical path (prevents both false rejections of legitimate in-root paths and missed escapes).

  2. Move containment checks outside try/catch β€” The resolved !== #realRoot guard now executes after the try/catch in both #resolvePath and #verifyAncestorInRoot, so rejection errors are never swallowed.

  3. Use EACCES for containment violations β€” More semantically correct than ENOENT; the path exists but access is denied by the sandbox policy.

  4. Add regression tests β€” Verify that read, stat, open, and write through an escaping symlink are all rejected with EACCES, and that writes never touch out-of-root files. Also covers the "create new file under an escaping directory symlink" path.

Testing

All existing and new tests pass and confirm the before vs. after behavior:

  • Before: operations through symlinks targeting outside the root succeeded silently (sandbox escape).
  • After: the same operations are rejected with EACCES; no data is read from or written to paths outside the root.

The containment check in RealFSProvider#resolvePath was thrown inside the
try block wrapping fs.realpathSync(). Because the rejection carried an
ENOENT code, the `if (err?.code !== 'ENOENT') throw err` catch mistook it
for "path does not exist yet" and returned the raw, symlink-containing
path, which the caller then opened -- following the symlink out of the
root. #verifyAncestorInRoot had the same self-swallowing flaw, making it a
no-op. As a result readFile/stat/open/write on a symlink pointing outside
the root read and wrote real files outside the sandbox.

Move the containment checks outside the try/catch so a rejection can no
longer be swallowed by the ENOENT handler, and compare against a
canonicalized root (#realRoot) so the checks are sound even when the root
path itself contains symlinked components (e.g. /tmp -> /private/tmp).

Add regression tests covering read/stat/open/write and new-file creation
through symlinks that resolve outside the provider root.
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem. labels Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.65517% with 6 lines in your changes missing coverage. Please review.
βœ… Project coverage is 90.24%. Comparing base (116302d) to head (f1acbb7).

Files with missing lines Patch % Lines
lib/internal/vfs/providers/real.js 89.65% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64409      +/-   ##
==========================================
- Coverage   90.25%   90.24%   -0.01%     
==========================================
  Files         741      741              
  Lines      241339   241366      +27     
  Branches    45476    45475       -1     
==========================================
+ Hits       217810   217822      +12     
- Misses      15084    15096      +12     
- Partials     8445     8448       +3     
Files with missing lines Coverage Ξ”
lib/internal/vfs/providers/real.js 95.91% <89.65%> (-0.17%) ⬇️

... and 23 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants