microsoft / vsmarketplace

Customer feedback and issue tracker repository for Visual Studio Marketplace
MIT License
39 stars 10 forks source link

Sanitise SVG instead of banning it #38

Open joaomoreno opened 3 years ago

joaomoreno commented 3 years ago

From @PeterWone in https://github.com/microsoft/vscode/issues/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.

luanpotter commented 3 years ago

This was also requested here: https://github.com/microsoft/vscode-vsce/issues/183

pokey commented 3 years ago

Maybe you'd consider supporting your own svg badges? I get an error for https://open.vscode.dev/badges/open-in-vscode.svg 😅

ghost commented 3 years ago

The SVG sanitizer at DOMPurify is meant to be enterprise grade, would love if you could use that :)

gjsjohnmurray commented 3 years ago

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

https://github.com/microsoft/vscode/pull/131950

ghost commented 3 years ago

Noice

ghost commented 3 years ago

As asked on vscode-vsce, is help wanted on this?