Allow SVG images to be uploaded in CKEditor

For image fields, see #1014816: Allow image fields to use any extensions the current image toolkit supports (instead of hard-coding jpg, png and gif only).

Closing dup to related issue.

As explained in https://www.drupal.org/project/drupal/issues/1014816#comment-12374785 this is not a duplicate because the focus here is to insert svg from within CKEDITOR (the falsly duplicate issue is about allowing image fields to upload any extension the installed toolkit is supporting)

Then I think it's better rescope the issue and move to the right component. For image *fields*, the referenced issue is the one.

CKEditor is only one way of providing input. What really matters, is whether it's safe to render this in text fields, whose stored data is passed through the filter module's text processing at render time.

So, moving to the text module component.

Um, perhaps I'm wrong but I'm unsure if this is really 100% correct. Yes, the text filter certainly plays a role in this. It should not filter out SVG tags once they are inserted into an HTML text field - BUT: Isn't it the ckeditor module that provides the "Insert image" button in CKEditor's toolbar and the accompanying dialog? Within this dialog, the user should be able to select SVG files in the first place. I don't think this is currently the case, or is it?

Within this dialog, the user should be able to select SVG files in the first place.

Uploading SVG images will likely never be supported, because that's inherently insecure. The only SVG support that could be considered, is embedding it in a text field, i.e. without it being a separate file.

I see now that the original request was about uploading SVG images. I thought #5 corrected that to be about embedding. Sorry.

See #1014816-41: Allow image fields to use any extensions the current image toolkit supports (instead of hard-coding jpg, png and gif only). This is also why Drupal.org doesn't allow SVGs to be uploaded: #1730120: Allow svg, eps, ai and indd file uploads via static file domain. I know, this all sucks, but it's because SVG's design is flawed wrt security…

BTW, if somebody does the work to A) prove it can be secure, B) write a patch, C) provide test coverage, then I'd be happy to get this committed.

But until then, I'm not going to be working on this — I already tried to get this in, and failed miserably :( But perhaps browsers have evolved and this is no longer a security issue? Research welcome :)

I think you are wrong. The confusion probably arises from the different ways an SVG can be inserted into the page. We're talking here about an SVG just being uploaded as a normal file and then being referenced (exactly like PNG, JPEG etc) in the IMG tag. No security problems arise from this AFAIK:

https://www.w3.org/wiki/SVG_Security

Quote: "Markup languages like HTML [...] can reference SVG as an image with the tag [...] If an SVG file is fetched as image, then certain requirements apply [...] Scripts must not be executed."

These restrictions make a lot of sense to me. They exist for this very reason. People really ought to be able to swap e.g a bitmap image with a vector image without worrying it might contain some JS. And obviously at least Chrome/Firefox handle it that way. Internet Explorer should be checked twice, though.

Ah, and in addition: If the stuff is inserted with an IMG tag, nothing needs to be modified from the text filter POV.

OK, investigating further, I found that SVGs can contain scripts that are executed, when they are called stand-alone (e.g. if you enter /sites/default/files/test.svg into the browser's adress bar). Scripts are not executed, when inserted via IMG tag. Not even in Internet Explorer (tested IE11, IE10, IE9). So this behavior seems to be consistent across browsers.

This means, that it poses indeed a problem if someone can upload an SVG and trick others into calling that direct URL. It can be avoided with .htaccess by checking the referrer, of course:

https://stackoverflow.com/questions/10236717/htaccess-how-to-prevent-a-f...

Thus... A great solution (IMHO) would be:
1) Add additional rules to the default .htaccess file provided by Drupal to prevent any SVG access without a referrer from the same site.
2) For added benefit one might consider adding CSP (Content Security Policy) headers to SVG files as well. This will help all browsers except IE.
3) Make SVG support configurable in settings.php, default to "No". Writing a big fat warning as a comment, that you must use a server that respects the .htaccess files for this to be secure.

Usually, if it was just another file format, I'd say it's perhaps too complex to be implemented, BUT: Vector images are very important. It's 2018 and the technology exists, and I think it's very sad that we still cannot use vector images as first-class citizens in Drupal. The above-mentioned .htaccess rule might even be benefical (independed of CKEditor) for users who allow this stuff to be uploaded somehow/somewhere on their site - unaware of potential problems.

For CSP, see also related #2868079: Add a default Content-Security-Policy-header for svg files.

Ah, *very* interesting, mondrake. The final comment says: "Content Security Policy is supported by IE 10+ (though as X-Content-Security-Policy in IE 10 and 11), so the only browser left out is IE9, which is no longer supported as of April 2017". So this means, CSP by itself would already be completely reliable for all supported browsers. I'd say this is the preferred solution. We don't need to mess around with HTTP_REFERERs at all then. Nevertheless, I'd still say it makes sense to have a setting/option for SVG uploads with a comment, that you have to make absolutely sure, that the .htaccess is actually respected by your webserver. One setting should apply to all uploads, of course. Not only for CKEditor. OK, what does it all mean? I'd say first: Bug #2868079 must be solved. Then, we need a global option in settings.php with appropriate warning/information. And finally, this bug here could finally allow SVG upload in CKEditor if the option is set. Such an option, BTW, is probably useful anyway, because not every site admin would perhaps want to allow users to upload SVGs (for whatever reason).

Just dropping by to say I would – potentionally – benefit greatly from this feature. If only because I already collected 100s of svg files, uploaded a good portion of those with the IMG field, but failed to make a proper view for them. And spent the entire afternoon trying to get CKEditor Image Upload to work instead. Honestly, I would be disappointed at its lack of svg support – if I wasn't so jaded already.

Going back now to try a different Views approach.

@zJoriz See #9. There are big security implications. I hate it too, but alas, this is the way the web works.

Yeah, Wim, you are right of course in stating that there are big security implications that have of course to be approached carefully which we are doing under #2868079. But I think your statement "Uploading SVG images will likely never be supported, because that's inherently insecure" was a bit too harsh, though. Consider: There are many different things in Drupal that need very close scrutiny for their potential security implications and nevertheless they have been solved eventually. And uploading SVG images isnt "inherently insecure". With proper CSP headers, it is in fact totally secure. The question is how we can verify whether they are configured properly.

And (perhaps!) you underestimate the importance of this. It's not just another image format. Since websites needs to span an extremly wide range of device resolutions these days, it is extremly important to be able to use vector graphics for icons, graphics etc. to get crystal clear images. Designers deliver those images and content editors want to upload them. So, even if it's a steep hill I wouldn't give up too early :-)

The funny thing is:
SVG images are required as default for the branding ...

CKEditor has been removed from core, CKEditor 4 is removed from Drupal Core in 10.0.0

FYI: the CKEditor 5 equivalent issue of this one is on the verge of landing: #3280279: Allow sites to programmatically opt in to accept more image type uploads in CKEditor 5: TIFF, SVG….