Sanitise SVG instead of banning it

PeterWone commented 3 years ago

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.

No browser I could find would allow this script to run when the image is loaded from the internet
When the image is loaded from the local filesystem by a browser, the scripts are loaded and can run but the same-source policy prevents them from doing much.

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.

vscodebot[bot] commented 3 years ago

(Experimental duplicate detection) Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

PeterWone commented 3 years ago

It is also possible to embed SVG base64 encoded in a DataUrl.

You could decode these and scan them for script tags. Any script so encoded is part of the extension, which itself is a script, but you might take the position that hiding a script in base64 encoded SVG is a bit suspicious.

mjbvz commented 3 years ago

It's a bug that svgs are being blocked in iframe based webviews. This is a duplicate of #118181

@bpasero The documentation the poster linked is about icons in the workbench. Leaving that part up to you

bpasero commented 3 years ago

This seems to be only about publishing to the marketplace (thus assigning @joaomoreno), not related to our built in SVG filtering. I think our marketplace is not accepting SVGs at all, e.g. for the logo or documentation.

However, the built in blocking of SVG was added by you @mjbvz and @kieferrm so you might want to follow up on the reasoning separately (via https://github.com/microsoft/vscode/commit/577105a72c08400f3311b92efefbe2a0fe23982c).

PeterWone commented 3 years ago

The marketplace doesn't accept SVGs and the given reason is security. I have all sorts of uses for scalable artwork unrelated to the marketplace, and if I can't bundle them it's a problem. Marketplace artwork is the least of my concerns. I can think of any number of ways to bypass the marketplace restrictions, but if it's really a security matter then it needs fixing. If the truth is that the real issue is that webview can't or won't support them then it might be a duplicate but I don't see how I could have known that.

joaomoreno commented 3 years ago

Moved to https://github.com/microsoft/vsmarketplace/issues/38

microsoft / vscode

Sanitise SVG instead of banning it #119713