-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sanitise SVG instead of banning it #38
Comments
This was also requested here: microsoft/vscode-vsce#183 |
Maybe you'd consider supporting your own svg badges? I get an error for https://open.vscode.dev/badges/open-in-vscode.svg 😅 |
The SVG sanitizer at DOMPurify is meant to be enterprise grade, would love if you could use that :) |
Looks like VS Code is switching to DOMPurify |
Noice |
As asked on |
From @PeterWone in microsoft/vscode#119713
The publishing page https://code.visualstudio.com/api/working-with-extensions/publishing-extension says that I can't ship SVG resource and they must be loaded from trusted sources "for security reasons".
I looked into why, and discovered SVG supported embedded or linked scripts with a script tag similar to the HTML script tag.
If SVG is a threat for VS Code then presumably VS Code fails to enforce the same source policy. If so, that's not well thought through. There is absolutely nothing stopping an extension from pulling files from the general internet and putting them into the file system after the extension is installed. For that matter you can't stop me from embedding SVG files as strings and saving them to the filesystem on first run.
If the risk is in the WebView pages then why not sanitise the SVG by the simple expedient of stripping all script tags and their content? Their mere presence reveals malice afoot so an alert to the user would not be out of place. Or even just look for
<script>
and barf on finding it.MalwareException: 'nasty.svg' contains script.
Disallowing SVG in extension bundles is not an effective threat mitigation. Sanitising could be effective if it were applied universally.
The text was updated successfully, but these errors were encountered: