prevent RealFSProvider sandbox escape via symlinks#64409
Open
VinzSpring wants to merge 1 commit into
Open
Conversation
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.
Codecov Reportβ Patch coverage is
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
π New features to boost your workflow:
|
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
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.
Summary
Fix a sandbox escape vulnerability in
RealFSProviderwhere symlinks pointing outside the root directory were silently followed instead of being rejected.Root Cause
The containment check (
throw createENOENT) was performed inside thetry { fs.realpathSync() }block in#resolvePathand#verifyAncestorInRoot. Because the thrown error carried anENOENTcode, the adjacentcatchhandler β 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
Canonicalize the root at construction time β
#realRootis resolved viafs.realpathSync()so that all containment comparisons use the canonical path (prevents both false rejections of legitimate in-root paths and missed escapes).Move containment checks outside
try/catchβ Theresolved !== #realRootguard now executes after the try/catch in both#resolvePathand#verifyAncestorInRoot, so rejection errors are never swallowed.Use
EACCESfor containment violations β More semantically correct thanENOENT; the path exists but access is denied by the sandbox policy.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:
EACCES; no data is read from or written to paths outside the root.