SVG uploads

[mxbclang]

• Module name: SVG Uploads
• Module slug: svg-uploads
• Module proposal issue: Module Proposal: SVG Uploads #410

High-level feature

Allow uploading of regular SVGs without scripts and provide an SVG preview in the Media Library.

Technical details

• This should likely use https://github.com/darylldoyle/svg-sanitizer
• This library requires at least PHP 7.0
  • Since uploading SVGs is not a critical feature to using WordPress, we can still use that library and build the SVG uploads feature in a way that it only works for WordPress sites on PHP 7.0+
  • WordPress sites on PHP 5.6 would simply not get SVG support, which is an acceptable tradeoff for this kind of feature

[mxbclang]

👋 @masteradhoc @alaca is working on research and reviewing the existing approaches for this one. He can also work on the implementation, but of course we'd love to get your input and thoughts as we move forward. Thank you again for your work on this!

[masteradhoc]

@bethanylang thanks!
@alaca would be glad to work together on this. contact me over twitter or slack to discuss how to move forward :)

[michael-sumner]

We will need to make sure that any script injection is taken into consideration. Especially considering the attacks in Web3 and the adoption of WordPress websites having Web3 integration.

A common one was the OpenSea.io hack by sending people free NFT's as SVG. Which would then pop up a request that asked the user to "confirm" logging into their crypto wallet, and then another "confirm" to complete the transaction of the NFT. It turns out that the "transaction" drained the wallet of funds by using JavaScript in the NFT... if the user would have read carefully.

The mass adoption of SVG module in WordPress could have JavaScript removed to avoid script injections, and offer an option to "Allow JavaScript in SVG uploads" with a clear "Warning:" sign.

[erikyo]

@michael-sumner Hope this fortinet post could provide some initial info about the svg security concerns.

However, I believe that using php for this does not provide 100% security for us, since the php XML handling is subject to xml injections itself. The best one in this case is DOMpurifier that is an industry standard but, also in this case, it would not provide 100% security in the WordPress environment because images can be uploaded in many ways and could bypass this control.

My personal solution (I made a plugin of that) was to create a block so that:

• Only editors can "upload" svg's
• Svg's are checked by DOMpurifier when they are uploaded or modified/saved or displayed
• The images are not stored in the media library and imagemagick doesn't process them // svgs are stored as an xml fragment inside the page where you added the block (as like the html block, I took inspiration from that)
• Bonus: since svg are included as an xml fragment in the page and are not a <img>, they can be edited (changing colors, adding borders, etc.) and animated (both css and js).

EDIT: this repo could be a very good starting point to test svg uploads

[adamsilverstein]

One possible security solution would be to only enable SVG embedding as images (not objects) - when the SVG is implemented as an image tag or CSS background image source, the browser will not execute any JavaScript embedded inside the SVG, reducing any security concerns.

[alaca]

Embedding SVG as an image will help prevent running JavaScript, but that will resolve only half of the problem. The worrying part of enabling uploads of SVG files is that SVG can potentially carry many types of malicious payloads and not just JavaScript. For example, anything from HTML Injection, XML Entity Processing, Denial of Service attack, and many more. Luckily, the svg-sanitizer prevents all attacks mentioned above.

Most of the plugins listed on WordPress.org that are adding the functionality to upload SVG to WordPress are using the same library to sanitize SVG files. Some of the plugins have 900.000 active installs, which means that this library is "battle-tested". I'd suggest using the same approach but simplified and rewritten to follow WPCS. Also, all existing tests can be reused.

[spacedmonkey]

It worth noting that svg-sanitizer library is only PHP 7.0+. WordPress core still supports 5.6. Either this library needs to be backported to php 5.6 or will have to be php 7+

[jeffpaul]

@spacedmonkey see darylldoyle/svg-sanitizer#80 (comment) for info on how we could get 5.6 compat added back to the svg-sanitizer library.

[adamsilverstein]

It worth noting that svg-sanitizer library is only PHP 7.0+. WordPress core still supports 5.6. Either this library needs to be backported to php 5.6 or will have to be php 7+

Another possibility would be enabling SVG only for sites that have 7.0+; we add conditional support for other formats based on server capabilities as well (eg. PDF, WebP).

[felixarntz]

Strong +1 to what @adamsilverstein; we also discussed this a few weeks back in one of the weekly performance chats, and everyone was aligned that this would be a good feature to simply only offer for sites on PHP 7+. It can easily be built in a way that SVG uploads are not allowed for those WordPress sites still running on PHP 5.6.

I've clarified that in the issue description now.

[mxbclang]

Additional discussion: WordPress/wporg-mu-plugins#300

[spacedmonkey]

Seems like this there is movement on making PHP 7.2 min version. Which would be extremely helpful - https://core.trac.wordpress.org/ticket/57345

[swissspidy]

Since we moved away from modules to standalone plugins, and there are already plugins like https://wordpress.org/plugins/safe-svg/ utilizing SVG Sanitizer, I don't see value in creating yet another plugin for SVGs. It could be worked on directly in core (in https://core.trac.wordpress.org/ticket/24251). Also, SVGs are not a priority for the performance team.

I suggest closing this issue here in favor of https://core.trac.wordpress.org/ticket/24251