fix: restrict customLibraries file paths to workspace folder#494
Open
theeggorchicken wants to merge 1 commit intohediet:mainfrom
Open
fix: restrict customLibraries file paths to workspace folder#494theeggorchicken wants to merge 1 commit intohediet:mainfrom
theeggorchicken wants to merge 1 commit intohediet:mainfrom
Conversation
The `customLibraries` setting accepts a `file` property that is resolved via `evaluateTemplate()` and read with `workspace.fs.readFile()`. There was no check that the resolved path stays within the workspace folder, so a malicious `.vscode/settings.json` could read arbitrary files on the host (e.g. `/etc/passwd`, SSH keys, cloud credentials). This adds a path containment check after template evaluation: the resolved path must start with the workspace folder. Absolute paths and `../` traversals that escape the workspace are now rejected with a clear error message.
Owner
|
Thanks for your investigation! However, I don't see how this could be turned into an attack. |
Author
|
The webview CSP in DrawioClientFactory.getOnlineHtml() sets connect-src: *, so any JS in the webview context can exfiltrate data via fetch() — and the online-url workspace setting lets an attacker replace the drawio UI itself with a page that captures all postMessage traffic, including the path-traversed file content. This is also hard to test overall! However the fix is 11 lines of code (and I've included some tests to show they are non breaking). |
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.
Security Fix: Path Traversal via customLibraries Setting
Vulnerable Lines
File:
src/Config.ts#L586-L590evaluateTemplate()only substitutes${workspaceFolder}- it performs no path containment. Any absolute path or../traversal is passed directly toworkspace.fs.readFile(). Since the extension declaresuntrustedWorkspaces.supported: true, VS Code does not warn the user when opening a workspace with these settings.What could happen
An attacker creates a repository with a
.vscode/settings.jsonlike:{ "hediet.vscode-drawio.customLibraries": [ { "libName": "x", "entryId": "x", "file": "/etc/passwd" } ] }When a victim clones the repo and opens any
.drawiofile, the extension reads the targeted file. Since the extension runs in the VS Code extension host process, it can read anything the user's VS Code process can: SSH keys, cloud credentials, environment files, etc.How to verify
settings.jsonshown above and a minimal.drawiofile.drawiofile/etc/passwd(an error will appear because the content isn't valid library JSON, but the file is read)Full reproduction traces: evidence gist
End-to-end validation was performed using
@vscode/test-electronwith VS Code 1.109.5 and vscode-drawio v1.9.0. A canary file placed outside the workspace was read by the extension after opening a.drawiofile, confirmed byatimechange on the canary (before:1771710897384, after:1771714497557).What this PR does
After
evaluateTemplate()resolves the file path, the resolved path is normalized viapath.resolve()and checked against the workspace folder root. If the path does not start with the workspace folder (plus a path separator, to avoid prefix collisions with sibling directories), an error is thrown with a clear message.This is consistent with how VS Code's own workspace trust model works: workspace settings should not be able to escape the workspace boundary.
Tests included
test-path-containment.js- six cases:/etc/passwd) is blocked../../etc/shadow) is blockedproject-secret/) is blockedNote: this project has no test runner infrastructure, so the test is a standalone Node.js script (
node test-path-containment.js).Evidence