? Pending

User tests: Successful: Unsuccessful:

avatar nikosdion
nikosdion
7 Apr 2020

Joomla's Media Manager does not support image and video formats which have been long supported by modern browsers.

References:

These file types, especially SVG, are pretty common in real world sites.

Since this will never make it into Joomla 3....

...I wrote SVG support as a plugin. It does in-memory patching of the necessary core files. It is opt-in; disable the plugin and poof! SVG support is gone. It does SVG sanitization, unlike Joomla 4 at the time of this writing.

It took, dunno, an hour? And most of that time was spent doing the in-memory patching and magic overloading of com_media options, not the actual technical solution to sanitize SVG uploads. Hardly the complicated technical solution that some people made it out to be.

That's the whole point of me writing this plugin. I wanted to prove it can be done, it can be done safely, it can be done quickly and it's not a massive, inscrutable feature you can't accept in Joomla 3 (while at the same time 3.10 is slated to have back-ported J4 code, for crying out loud!).

For me it's now completely irrelevant if Joomla will ever add SVG support. I can EITHER use my plugin OR JCE Editor Pro to support SVGs on my sites. I put my code where my mouth is. kthxbye

Summary of Changes

WebP and SVG are recognized as image formats. Therefore they can be previewed inline in the
Media Manager page and selected in an image selection form field.

WebM is recognized as a video format in the Media Manager.

Testing Instructions

  • Go to Content, Media
  • Try to upload an SVG file (TEST 1, see below)
  • Click on Options
  • In "Legal Extensions (File Types)" add svg
  • In "Legal Image Extensions (File Types)" add svg
  • In "Legal MIME Types" add image/svg+xml
  • Click on Save & Close
  • Upload an SVG
  • TEST 2 (see below)
  • Create a new article in Content, Articles, Add New Article
  • Go to Images and Link
  • In the "Intro Image" click on Select
  • TEST 3 (see below)

Important note: even though this PR adds SVG, WebP and WebM as legal extensions, legal image extensins and legal MIME types these changes will NOT take effect if you are testing this PR through the Patch Tester or if you are otherwise applying it on an existing site. Hence the extra steps about going into the Options page. These changes will only be applied on NEW sites only.

Expected result

Test 1: I am able to upload an SVG file

Test 2: I am able to see the SVG file I uploaded as an inline preview in Media Manager

Test 3: I am able to select the SVG file I uploaded in the Media Manager

Actual result

Test 1: I can NOT upload an SVG file

Test 2: No inline preview. A file type icon is shown.

Test 3: I can NOT select the SVG file; it's not recognised as an image.

Documentation Changes Required

None.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar nikosdion nikosdion - open - 7 Apr 2020
avatar nikosdion nikosdion - change - 7 Apr 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 7 Apr 2020
Category Administration com_media
avatar brianteeman
brianteeman - comment - 7 Apr 2020

I'm all for webp and webm being added

the discussion on svg support being added by default has been discussed many times before

avatar nikosdion
nikosdion - comment - 7 Apr 2020

Well, the only vector format supported by browsers is SVG. We can of course tell our site integrators to shove it and have to enter the paths to their SVGs manually but this is hardly a workable, modern CMS. No wonder people are flocking to WordPress.

avatar brianteeman
brianteeman - comment - 7 Apr 2020

We can of course tell our site integrators to shove it and have to enter the paths to their SVGs manually but this is hardly a workable, modern CMS. No wonder people are flocking to WordPress.

They wont get svg in wordpress either (unless something just changed) https://www.wpbeginner.com/wp-tutorials/how-to-add-svg-in-wordpress/

avatar nikosdion
nikosdion - comment - 7 Apr 2020

Regarding WordPress, I am not making comments about WP vs J lightly. On my WordPress 5.4 local site I use for development I can upload SVG files to the Media Library and I can insert them just fine in posts using the Block Editor (Gutenberg) by selecting them in the Media Manager, uploading them directly or linking to them by URL. I actually tested it AGAIN just now. Go ahead, download and install WordPress using their "famous 5' installation" and give it a whirl.

Now let's see in my Joomla 3.9.16 site of the same vintage. I can't even upload SVGs to the Media Manager. I need to know which THREE (3) options to change. Even if I do that, I cannot insert them in articles or select them anywhere else where images are allowed. I can only link to them by URL.

Here are some further thoughts about image file support in Joomla.

The media manager currently supports XCF and will even try to display an inline preview. As far as I can tell from searching the MDN it's not supported by browsers. Maybe it was supported circa 2004 when Mambo was around. If memory serves, it was supported only on Firefox on Linux. Admittedly, 16 years later it's hard to be sure.

WebP is not supported on mobile Safari. Its utility is very limited for that reason, especially since Joomla doesn't have any native support for the <picture> tag when selecting images using the Media form field. Therefore you can't realistically use WebP using the image picker because it would be useless. In the WYSIWYG editors you can always key it in manually in a picture element, meaning this is probably a useless change except for simply being able to preview your WebP.

WebM, like MP4, cannot be previewed and Joomla does not have a video select field. As far as I can tell you can't insert a video object using TinyMCE unless you drop to source view. So that's just a meaningless change put there just in case something changes.

SVG, however, is supported by all modern browsers, it can be previewed inline and it is tremendously useful for people building real world sites, it being the only vector format supported in modern browsers and really small nonetheless. Moreover, everything can be addressed by icon fonts and not everyone is like our company, creating custom icon fonts for their sites.

I do get the security issues regarding SVG support. However, this is a rather bogus argument when I have already contributed MediaHelper::canUpload since Joomla 3.4. This code, which was part of my Admin Tools Professional security solution, will completely block the upload if the file contains a <script> tag. Therefore it addresses the security concerns of SVGs.

Can you manually upload malicious SVGs if you override Joomla's upload protections or if you use FTP/SFTP/your host's file manager? Sure you can... in the same way you can upload malicious JavaScript and PHP files on a site you have total control over i.e. it falls under the category "I can hack myself".

avatar coolcat-creations
coolcat-creations - comment - 7 Apr 2020

I have tested this item successfully on 2c00477

Tested successfully according to the description with svg, webp and webm videoformat


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28599.

avatar coolcat-creations coolcat-creations - test_item - 7 Apr 2020 - Tested successfully
avatar brianteeman
brianteeman - comment - 7 Apr 2020

I do get the security issues regarding SVG support. However, this is a rather bogus argument when I have already contributed MediaHelper::canUpload since Joomla 3.4. This code, which was part of my Admin Tools Professional security solution, will completely block the upload if the file contains a <script> tag. Therefore it addresses the security concerns of SVGs.

I wonder why svg has always been rejected then

avatar nikosdion
nikosdion - comment - 7 Apr 2020

@brianteeman I can tell you exactly why it was always being rejected. But I need to take the scenic route to get there.

SVG files can have JavaScript. This used to be a problem. The keyword here is "used to". The article I linked you to is from ten years ago ?

In the meantime, certain things have changed.

Since Joomla 3.4 we have the code I mentioned which detects scripts in any uploaded file and rejects it from ever being uploaded. Of course this is not a panacea because an enterprising miscreant could plausibly use event attributes. This will become irrelevant in Joomla 4 with content security policy support; the default settings prevent inline event attributes unless the resource uses the CSR token which is not the case with <img> tags.

But what about Joomla 3 and right now? I have good news that people may have not yet heard.

Modern browsers do not allow script execution in SVG files when they are included in <img> tags. You'd have to include your SVGs as an <object> which can only be done by manually writing HTML code in the WYSIWYG editor's source code tab so it's another case of "I can hack myself".

If I recall correctly that last change about SVGs in IMG tags not executing code only took place circa late 2017 for the security reasons I linked to at the top of this reply. All the discussion in Joomla was based on the outdated assumption that an SVG loaded through an IMG tag would be a security nightmare. This is simply no longer the case.

So, to put it clearly: the only reason Joomla has consistently rejected SVG support the last 2 ½ years is that the people making these decisions have stale security knowledge. Hopefully we can now shift the discussion to how browsers work in the real world, today versus an outdated mental model that's not been relevant in ages.

avatar brianteeman
brianteeman - comment - 7 Apr 2020

Are you sure you don't have something extra installed on your wp install. I just checked with a copy of 5.4 that only has backup installed and I cannot add svg images

wp

(Or maybe it was because the only svg I had to hand was the joomla logo :) )

avatar nikosdion
nikosdion - comment - 7 Apr 2020

Stock WordPress 5.4 with Yoast. This is the file I used: https://svgstudio.com/pages/free-sample

avatar brianteeman
brianteeman - comment - 7 Apr 2020

Maybe it is something yoast is doing then because I still cant upload svg

image

avatar SniperSister
SniperSister - comment - 7 Apr 2020

Nicholas is right, an SVG being referenced in an -tag can hardly do any harm, so what's the threat scenario here?

SVGs, if embedded directly, in an object-tag or if opened directly, can execute JS either in an <script>-Tag or using one of the gazillion on*-event attributes. So, if you i.e. get a user to click on a link to a manipulated SVG file, you will end up with a script execution.
Now, let's think about a low-privilege user (i.e. editor) who is uploading such a manipulated SVG with an onmouseenter attribute and is sending that link to the site's super administrator with the instruction to "quickly look at that image because it has a glitch in Firefox". The super admin clicks on that link, JS is executed in the super admin's browser session, allowing the JS to execute whatever task with super administrator privileges – in that case, we can use the script to upgrade the editor's account to a full privilege account, resulting in a full compromise of the site.

The current HTML filter logic in Joomla does it's very best to prevent exactly that scenario by filtering malicious tags and attributes in non-superadmin input. With SVG upload being added and being enabled by default, that layer of defense is significantly weakened.

I'm personally open for an opt-in support (just like users can chose to allow <script>-Tags for non-superadmins, but a default-on setting is a no-go.

avatar brianteeman
brianteeman - comment - 7 Apr 2020

I'm personally open for an opt-in support (just like users can chose to allow <script>-Tags for non-superadmins, but a default-on setting is a no-go.

That's what we have right now isn't it? You optin by adding the mime types etc

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

I believe that it is wrong to forbid everyone to upload. I think that it is necessary to bind to rights. That is, to limit the upload of file types by user rights and, accordingly, make not one input input, but several broken into rights. By default, for administrators and a super user, allow svg, prohibit others and the administrator himself can limit what to download and to whom.

avatar SniperSister
SniperSister - comment - 7 Apr 2020

I'm personally open for an opt-in support (just like users can chose to allow <script>-Tags for non-superadmins, but a default-on setting is a no-go.

That's what we have right now isn't it? You optin by adding the mime types etc

You’ll still lack the changes to the model that are added in this PR

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

I recently did file uploads on the front. And I realized that I can’t specify which user groups can upload which files. I think you need to expand all the same settings.

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

I will soon do this in my file manager and will be able to demonstrate this in work.

avatar brianteeman
brianteeman - comment - 7 Apr 2020

As far as I can tell you can't insert a video object using TinyMCE unless you drop to source view.

You can use the tinymce media button - admittedly its not that friendly but it does work

avatar nikosdion
nikosdion - comment - 7 Apr 2020

Can I propose a common sense, pragmatic AND positive user experience approach?

Have the Media Manager recognize as images the extensions configured as images in, um, the media manager’s options. This is what users expect based on my observation of people getting confused as to what that option is meant to do anyway.

If you’re interested I can do this by Friday, depending on work volume this week.

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

@brianteeman why not just expand the settings? what are the problems?

avatar brianteeman
brianteeman - comment - 7 Apr 2020

I just echo what the JSST lead said and that no cms supports svg out of the box

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

JSST who is it? how to decipher.
I just can’t understand why it’s impossible to expand the settings and make adequate presets for this. Where are the facts that it is unsafe in this format.

avatar brianteeman
brianteeman - comment - 7 Apr 2020
avatar nikosdion
nikosdion - comment - 7 Apr 2020

@brianteeman I missed some of your earlier messages. Thanks about the video embed, it’s REALLY well hidden ? I was just using JCE by default.

Regarding the Media Manager options, they are a weird little case and I’ll have to get technical (sorry). All three options in the test instructions? They control uploads. Only. They don’t control image selection. The image selection is hard-coded in the model I modified in this PR.

What user expect – even experienced ones like my wife – is that the option about image file extensions has some effect on which files can be selected in the Media field. This is currently not the case :/ Making SVG (and JPEG2000 and you-name-it) support opt-in would require modifying the model to use the extensions defined in the media manager as the accepted extensions for image files. In case it’s not set we can use the current default list. This is necessary because the Media field’s layout simply loads the images view of com_media.

This would still be a problem in the case that @SniperSister explained BUT it’s opt-in and you only get to do that if you understand what you’re doing. Also, I would posit that the number of practical use cases where your Manager user can be an untrusted scoundrel is rather low. Also, it’s not a practical attack. The Manager would need to create an object or embed tag – which CAN be filtered in Joomla’s Global Configuration – to have code executed. If the Super User opens the file directly in a browser he runs untrusted JS in the context of a blank browser window which is the baseline threat level when visiting any random Internet site with JS enabled, i.e. the default threat level for basically everyone. And if we want to really be pedantic about it we CAN implement code that goes through the SVG structure and removes script tags and inline event handlers.

I can implement either or both of these suggestions BUT I need to know if they are going to be accepted before I start working on them. The reason is that we’re talking about a fair chunk of my time that I am only willing to allocate to core contribution if there’s a fighting chance of my contribution seeing the light of day. Otherwise it’s far easier for me to do a one-line core hack on the sites I need to support SVGs (in the same way that it’s easier for a tight-rope artist to go between two buildings balancing on a tight rope than getting the elevator down, crossing the street and taking the other elevator all the way up, i.e. whoever is reading this, DON’T TRY THIS AT HOME).

avatar SniperSister
SniperSister - comment - 7 Apr 2020

If the Super User opens the file directly in a browser he runs untrusted JS in the context of a blank browser window which is the baseline threat level when visiting any random Internet site with JS enabled

Nope, that was my point: if the JS in the SVG is executed in the origin context of the hosting domain, so a JS hosted in a Joomla site can i.e perform an AJAX call that has access to the current session cookie. It’s not a isolated context.

avatar nikosdion
nikosdion - comment - 7 Apr 2020

AFAIK in this case you don’t have a login cookie unless your images folder is in the administrator folder (bad idea) or you use unified sessions (also a bad idea).

avatar SniperSister
SniperSister - comment - 7 Apr 2020

@nikosdion I did a quick test with an SVG in /images and accessing the administrator interface seemed to work at the first glance, but I’ll do a proper test case tomorrow and post an update here

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

AFAIK in this case you don’t have a login cookie unless your images folder is in the administrator folder (bad idea) or you use unified sessions (also a bad idea).

There is

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

@SniperSister You described if the average user loads. If you expand the settings and each input for other group from acl. and make presets, then the situation will not work.

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

@nikosdion if the admin is authorized in the admin panel and the scripts from svg crawl and his session is active, then the script from svg will get access

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

@SniperSister or do you equate an ordinary user from a group with an administrator?

avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

I just echo what the JSST lead said and that no cms supports svg out of the box

@brianteeman
Then, joomla will be the first. Svg needs to be uploaded and ways have to be invented.

avatar nikosdion
nikosdion - comment - 7 Apr 2020

OK. I’ll keep doing my single line core hack.

avatar nikosdion nikosdion - close - 7 Apr 2020
avatar nikosdion nikosdion - change - 7 Apr 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-04-07 20:11:26
Closed_By nikosdion
Labels Added: ?
avatar dmitriitux
dmitriitux - comment - 7 Apr 2020

@nikosdion why did you close, let's just discuss here and find a solution? why close?

avatar nikosdion
nikosdion - comment - 7 Apr 2020

I closed the PR for two reasons.

One, it's not an opt-in solution to SVGs. IF you have a site with untrusted media contributions AND the Super User open an SVG outside an image tag THEN we have a potential security problem. Therefore this PR doesn't cut it for this use case. BTW, many sites out there tend to be one to a handful of very trusted Super Users which is what I had in mind. Sites with untrusted contributors are not all that common but they do exist.

Now the second reason is that even though there is a technical solution I don't see any willingness to discuss this, hence my previous comment. I see it was misunderstood. I can expand the point a bit further.

Being exposed to a malicious SVG is irrelevant if you have neutralized JavaScript in the SVG or simply rejected uploading it.

Sure, Joomla's HTML filtering sucks. It's a naive, home grown solution maintained in a haphazard fashion. We should really be using HTML Purifier instead. That's what I had to use when I absolutely needed to be pretty darned sure that the HTML I was getting from emails would not contain dangerous junk. Works like gangbusters. Maybe we should do this for Joomla 4. Or 5. But I digress.

SVGs are XML, not plain old SGML like HTML. XML can be reliably enumerated, node by node and attribute by attribute. We've kind been doing that since before this project was formed ? Also, unlike HTML, SVG has a finite and well defined set of tags and attributes which can contain JavaScript. This means that we could either reject SVGs which contain them or even remove them to neutralize them. The former is faster but more precarious (because RegEx), the latter is slower but safer (because SimpleXMLElement).

Moreover, SVG support would be opt-in anyway. Not in some fuzzy, wishful thinking way but with a realistic solution that doesn't even require more options in Joomla: using the image extensions option in the Media Manager, what users actually expect this option to do.

I would really want to discuss these technical solutions and possibly implement them. I've already stated that clearly. However, if I am receiving absolutely no comment on a technical solution that requires a fair amount of time to implement I am very reluctant to spend the time to implement it. If I'm told after the fact that it doesn't satisfy the project I've simply wasted my time. In that case I'm better off using my one line core hack both for time efficiency and mental health reasons.

So, if there is someone from the project who would like to have a technical discussion about what I proposed I'm here and all ears. But I'd really like to discuss this solution, not what would happen if we didn't implement it because that point is a bit moot. Thank you.

avatar b2z
b2z - comment - 8 Apr 2020

@nikosdion why not to leave at least webp and webm support in this PR?

avatar nikosdion
nikosdion - comment - 8 Apr 2020

I already explained that earlier.

avatar coolcat-creations
coolcat-creations - comment - 8 Apr 2020

Please reopen it, it would be great to find a solution to make it possible.
If svg is not allowed we would need to disallow creating files in the Template Manager too? Besides that it’s important for UX like you wrote in the beginning.

avatar C-Lodder
C-Lodder - comment - 8 Apr 2020

I don't get it. Joomla won't support SVG's, but they'd allow someone to submit an extension to JED that:

  • Contains malicious SVG's
  • Is prone to SQL injections
  • Fetches all user emails from the database and sends them to a remote destination
  • Hijacks the username/password input fields and sends keystrokes to a remote destination

You're more at risk when using an extension from JED than you are uploading an SVG via the Media Manager.

avatar infograf768
infograf768 - comment - 8 Apr 2020

You're more at risk when using an extension from JED than you are uploading an SVG via the Media Manager.

That is a serious statement. Examples?

avatar C-Lodder
C-Lodder - comment - 8 Apr 2020

@infograf768 I'll happily talk privately about this

avatar nikosdion
nikosdion - comment - 8 Apr 2020

@coolcat-creations Based on the small nuggets of technical discussion we had in this PR I am not satisfied with my PR. It doesn't address either the opt-in approach or the security tightening when people do opt-in. I know exactly how to make it happen but it's a fair amount of my time (I reckon about 2-3 days plus another 20-50 hours of my time wasted in discussions around it before it's merged or rejected) and I am unwilling to do it unless someone official comes here and says "yes, Nicholas, we are interested in this".

Regarding the template overrides, it's false equivalency. The attack mode in this case is "I can hack myself". Regarding template overrides, uploading SVGs outside Joomla etc – they can only take place by TRUSTED site managers with the "keys to the kingdom" (full disk read and write access). If I am a top level trusted site manager I can hack myself; that's tautological.

What @C-Lodder discussed is worth a bit picking apart. The JED submission process DOES NOT vet an extension for security. Last time I checked, about two years ago, this was stated in the JED ToS. So, yeah, it is possible that a sneaky developer can put malicious PHP or JavaScript code as well as media (JPG, PNG, SVG, PDF, ...) that contains malicious code or triggers a vulnerability in the users' systems. This is also true to some extent for all app stores, be it for popular CMS or for popular mobile and desktop OS.

You can NOT realistically vet every single bit of third party code when submitted or updated. Code audits take months and cost dozens of thousands of Euros. Per extension, per published version. The fact that a trillion dollar company won't do it should tell you a lot about the feasibility of something like that.

However, even that argument is a false equivalency. You can NOT have an untrusted user (Manager, Registered or even Guest!) upload and install an extension in Joomla. You can, however, have them upload an SVG, an EXE, a .DOC (legacy format with macros enabled), .DOCM (relatively safer because in theory Office asks you to enable macros; in practice it has the occasional vulnerability) and so on and so forth. We treat our users as grown ups who know WTF they are doing.

Except when it comes to SVGs. Then we are a bunch of patronizing, sons of guns.

We do allow site owners to permit the upload of SVGs by modifying the Media Manager options. We do NOT allow them to select it in the Media field which is used to generate IMG tags in the HTML output which are not a security problem as I discussed. Instead, in this case, we require them to type in the path to the SVG in the Media Manager. Even worse, their way of finding which SVG to choose (since they don't have an inline preview in the Media Manager) is to copy the URL of each SVG they think might be the one they are looking for and paste it to their browser so they can open it for preview. As @SniperSister pointed out, that CAN be a security issue.

Therefore our insistence on not supporting SVGs at all leads to a situation that's more harmful to our users than if we supported SVGs in the Media Manager ?

But as I said earlier, in light of @SniperSister and @dmitriitux comments about SVGs opening in the browser I wouldn't in good conscience consider blindly allowing SVGs in the Media Manager a complete solution, fit for inclusion in the core (hence closing this PR). Even if it's opt in it still leaves a window of opportunity for a Super User to follow the same insecure behavior they do today (open SVGs for preview in the browser). That's why we need a technical solution to EITHER forbid the upload of potentially unsafe SVGs OR neutralize JavaScript in SVGs.

After a good night's sleep I think that preventing uploads is the best solution, despite what I said last night. In some cases Super Users might actually need animated SVGs from trusted sources they can include in OBJECT tags. If we silently remove JS from SVGs people will be frustrated that uploading an SVG through Joomla seems to "break" it and are likely to resort to risky alternatives such as installing direct upload scripts with zero checks ? Blocking the upload with a message similar to "This SVG contains executable code and cannot be uploaded through Joomla for security reasons. If you absolutely trust this SVG please upload it with SFTP, FTP or your hosting control panel's file manager." it will be far more user friendly.

Since I get no feedback from the project I will just add the technical solution for SVG uploads to my paid security software. I might even come up with a solution for letting you select SVGs in the Media field (I have a couple of ideas up my sleeve). I tried to do the right thing first. Now it's time to get back to business.

avatar SniperSister
SniperSister - comment - 8 Apr 2020

AFAIK in this case you don’t have a login cookie unless your images folder is in the administrator folder (bad idea) or you use unified sessions (also a bad idea).

I took a deeper look: any AJAX request target at the administrator folder will indeed use the admin session token, regardless where the image is located in the site's folder structure.

If svg is not allowed we would need to disallow creating files in the Template Manager too?

Template-Manager is super-admin only, that's a whole different scenario.

You're more at risk when using an extension from JED than you are uploading an SVG via the Media Manager.

Of course. Each and every extension is a significant security risk, that's why you should chose your extensions wisely and use as few extensions as possible. However, I don't see why that is a reason to deliberately add a XSS attack vector to core.

SVGs are XML, not plain old SGML like HTML. XML can be reliably enumerated, node by node and attribute by attribute. We've kind been doing that since before this project was formed ? Also, unlike HTML, SVG has a finite and well defined set of tags and attributes which can contain JavaScript.

For the specification side of things that's true and seems straightforward: validate the XML structure, filter script-tags and event attributes and you are ready to go, right?
In theory: yes. However, our experience with our "home grown solution maintained in a haphazard fashion" and the reported issues related to it, show that our main issue are the clients parsing the markup. These clients are built to be "fault tolerant", parsing markup in an unexpected way. Quick example:

<svg version="1.1" baseProfile="full" width="300" height="200" xmlns="http://www.w3.org/2000/svg"> <text x="150" y="125" font-size="60" text-anchor="middle" fill="red"><a href="java&#009;script:alert(1)">SVG</a></text> </svg>

That's perfectly valid SVG, is using "secure" tags and attributes and willstill trigger the alert on click in current Firefox and Chrome editions (haven't tried more browsers). Even if a filter looks for 'javascript:' in href tags, it will miss that specific case because of the encoded tab character in the string.
That's just one edge case, but those quickly add up to a long list that one has to have in mind - and the risk of missing cases is high.

Again: don't get me wrong, I would love to see SVG support in core and I agree that at least the UX issue needs to be fixed. The better option (filtering or at least blocking malicious inputs) would be the best solution from an UX perspective and using a 3rd party solution (see https://github.com/darylldoyle/svg-sanitizer) that already covers a lot of the edge-case scenarios is probably our best chance - however we have to be aware that we'll very likely miss edge cases, turning that UX-improvement into a XSS vector.

So, at the end of the day it's about balancing the remaining risks and the UX benefits.

avatar nikosdion
nikosdion - comment - 8 Apr 2020

Side point: The HTML filter (off-topic; ignore or let's make a new issue)

However, our experience with our "home grown solution maintained in a haphazard fashion" and the reported issues related to it, show that our main issue are the clients parsing the markup.

When writing Akeeba Ticket System I wanted to allow people to file tickets with a WYSIWYG editor or through email. A few simple tests later I was pulling my hair. It allowed a lot of XSS slip unless you had configured it to only allow basic tags (p, b, i, strong, em) without attributes. I needed links and images, so...

Your problem isn't that the SGML parser in modern browser is "fault-tolerant" (which is a requirement for an SGML parser) but that Joomla's HTML filter code is not at all robust. I'm not saying that Joomla core devs are incompetent, I'm saying that when your scope is an entire freaking CMS' front- and backend you can't excel in all aspects of its codebase.

If JSST would like to improve HTML filtering it should consider using a real HTML filter like HTML Purifier. In my tests this worked best. Probably because these people focus on writing just an HTML filter, not an entire CMS.

About not receiving reports, you have fallen into the classic pitfall: lack of reports to the contrary is not a proof of security. I remember that when I was in the JSST we were getting quite a few reports about it with regards to content submission from the frontend.

The reason you no longer have any reports about XSS is that the default settings disallow all HTML for Guest and Registered users (and, by extension, the user groups most people create on their sites). Most site owners don't even know how to change text filtering, even though it's been there for years and documented; look at the forum. While you're at it look at the horrible advice people give i.e. "yeah, just disable filtering".

Moreover and as I said, most sites only have a few very trusted high-privileged (Administrator and Super User) users where filters don't apply but also fall under the category "I can hack myself". So, yes, the default options in Joomla and its real world usage pattern means that virtually nobody is exposed to the failures of its HTML filter therefore nobody is reporting anything. If you need to have a site which allows frontend submission of HTML from untrusted sources you're screwed unless the 3PD of your extension took some extra steps to prevent such mishaps.

SVGs are XML and can be parsed

The second point I have already made is that SVGs are meant to be valid XML. We would NOT be filtering with Joomla's HTML filter. What I talked about was full parsing of its DOM which, by the way, is exactly what the sanitizer you linked to does, in the way I described. He's already gone through the SVG specification so there's no need to duplicate that work. Awesome!

Regarding using a sanitizer you probably missed my previous comment: for UX reasons it's best to prevent upload than to sanitize. However, considering how difficult it is to detect problems vs sanitize I am open to sanitization.

Most important point: Joomla 4 already supports SVGs as an opt-in feature ?

however we have to be aware that we'll very likely miss edge cases, turning that UX-improvement into a XSS vector.

What we're discussing would be better than what Joomla 4 currently does. While we're here beating ourselves to a pulp over whether we can add a single line in the core to allow SVGs to be picked in the Media field IF the site owner deliberately enables SVG support with the three Media Manager options I described at the top of the PR, Joomla 4 already does that.

Fire up J4. Edit the Media Manager options I said. You can now upload SVGs and select them in the Media field, e.g. as an article's intro text.

So much for this thread here... ?

How can we fix this mess

For all I see, the only way for Joomla 3 and 4 to have SVG support parity and address the SVG security concerns here is what we have to do:

  1. Joomla 3: Add support for SVG, WEBP, WEBM per my original PR. However, we DO NOT add SVG support in the default options; a site owner has to explicitly opt into them.

  2. Joomla 3: Update the media upload helper to sanitize SVGs. We can use the sanitizer you linked to.

  3. Joomla 4: Update the media upload helper to sanitize SVGs, like in Joomla 3.

If the JSST agrees I would like to make 2 PRs. One for points 1 and 2 for Joomla 3. Another one, after the first is merged, for the third point in Joomla 4.

avatar HLeithner
HLeithner - comment - 8 Apr 2020

From my point of view I would really like to see a svg solution but only if it's pulled proof. About point 1 and 2, point 1. is a new feature and are not allowed in 3.x series and I try my best to follow SemVer even if some people complaint that I miss classify a PR.

For 3. (J4) upgrading our filter technique with HTML purifier and svg-sanitizer would make sense to me but that's only my opinion.

avatar nikosdion
nikosdion - comment - 8 Apr 2020

@HLeithner Wow, that's some HELLUVA selective application of semantic versioning!!! Look at 3.0.0. Look at 3.9.16. Now please seriously tell me that the have feature parity and I will show you an idiot. Of course we add features in major releases. We do NOT follow semantic versioning in Joomla. We never did and we never will. Otherwise ever two of what our now sub-minor versions we'd have a new major version. We'd be at Joomla 434 or something ridiculous like that. What Joomla follows is a promise that within a major version nothing big breaks and within a minor version nothing, even minor, should break. When breakages happen we end up releasing a new minor or sub-minor version and call it a day.

OK, I get it. The project is not interested in SVG support. I'm just mad at myself for wasting my time, AGAIN. When will I ever learn that Joomla is not conducive to reasoning? Dammit, I am such an idiot.

avatar brianteeman
brianteeman - comment - 8 Apr 2020

Wow, that's some HELLUVA selective application of semantic versioning!!!

I think what @HLeithner is referring to the policy of no new features in 3.x at all and if we follow semver this is a new feature and therefore hits that policy and not to a general understanding of semver

avatar coolcat-creations
coolcat-creations - comment - 8 Apr 2020

OK, I get it. The project is not interested in SVG support. I'm just mad at myself for wasting my time, AGAIN. When will I ever learn that Joomla is not conducive to reasoning? Dammit, I am such an idiot.

I am personally very interested in svg support as a user of Joomla. Also I think some lots of others are interested in this. Just because some disagree it's not a reason to throw it into the trash. Can we please figure out with JSST how to integrate it into Joomla so that everyone is happy :-)

avatar HLeithner
HLeithner - comment - 8 Apr 2020

It's about 3.9.x bugfix releases not about the 3.x minor versions

avatar dmitriitux
dmitriitux - comment - 8 Apr 2020

I agree with @nikosdion.
But I insist that it is necessary to make advanced settings who can download file types from rights (ACL), because it applies to everyone.
For instance. I do not need managers to upload pdf, but the administrator needs to uploaded. And this cannot be decided by the standard manager.
I think it is necessary

  1. expand the settings, that is, do as the "Rights" tab
  2. implement an additional filter for svg and make it an option that turns on.
  3. Prepare and set up migration and presets.
avatar dmitriitux
dmitriitux - comment - 8 Apr 2020

@SniperSister if there is a template editor, then svg of any type can also be uploaded. And just to solve all these problems, you need to make advanced settings. And tell each group from acl that it can upload and apply additional filters

avatar crystalenka
crystalenka - comment - 8 Apr 2020

From a usability perspective, it is not a new feature - it's a fix. It makes the media manager behave in a way the user expects it to. As it stands now, it feels broken, even if it's technically not.

avatar Fedik
Fedik - comment - 8 Apr 2020

can we at least just fix the com_media/model/list to allow display the svg,webm files in the modal select?
I can upload via ftp, that all do, but it annoying that it cannot be selected

avatar crystalenka
crystalenka - comment - 8 Apr 2020

^^^^ That's exactly what I mean. :)

avatar dmitriitux
dmitriitux - comment - 8 Apr 2020

can we at least just fix the com_media/model/list to allow display the svg,webm files in the modal select?
I can upload via ftp, that all do, but it annoying that it cannot be selected

I think, for good in the preview you need to create thumbnails, and in svg cut out all script tags and animations

avatar crystalenka
crystalenka - comment - 8 Apr 2020

I think, for good in the preview you need to create thumbnails, and in svg cut out all script tags and animations

When you display an svg in an img tag, like the preview does, it does not animate nor run script tags.

avatar dmitriitux
dmitriitux - comment - 8 Apr 2020

I think, for good in the preview you need to create thumbnails, and in svg cut out all script tags and animations

When you display an svg in an img tag, like the preview does, it does not animate nor run script tags.

If displayed as img tag, then yes. Well, I wrote what I think needs to be done is to expand the settings, do additional filtering and, as @nikosdion says, review the HTML filter altogether.
And if possible, add thumbnails.

avatar crystalenka
crystalenka - comment - 8 Apr 2020

If displayed as img tag, then yes. Well, I wrote what I think needs to be done is to expand the settings, do additional filtering and, as @nikosdion says, review the HTML filter altogether.
And if possible, add thumbnails.

Yes, but all of that is moot if the PR won't be merged in Joomla 3 because it's considered a "feature". A bare minimum "fix", for users, would be allowing you to select/preview the svg in the media manager.

avatar alikon
alikon - comment - 8 Apr 2020

if i can spend some words, here, i really want this can happen in j3, even if i'm not so expert or not at all in the security matter, but for an end-user this is perceived like a needed feature, so let's make this happen, without loosing the security, i hope it cannot be so difficult if everyone put his ego in suspended mode for sometimes

avatar progreccor
progreccor - comment - 8 Apr 2020

we really need SVG support in Joomla 3!

avatar TobsBobs
TobsBobs - comment - 8 Apr 2020

I need also this feature in Joomla. Please make it happen.

avatar dgrammatiko
dgrammatiko - comment - 8 Apr 2020

OK, I get it. The project is not interested in SVG support. I'm just mad at myself for wasting my time, AGAIN. When will I ever learn that Joomla is not conducive to reasoning? Dammit, I am such an idiot.

@nikosdion there is no salvation for this project. I'm trying to add svg support for some years without any success:
2017: #13499
2018: #21410
2020: #28351

Joomla is seriously outdated but the key people are still in denial...

avatar alikon
alikon - comment - 8 Apr 2020

no please no that's not correct
if you really want to change things ..... you cannot do it from outside
that's was my understanding...

i really want to change things , the only way i've found, is to fight things from inside, and even then it's quite difficult
i'm successfull ?
maybe no,
but at least i'm still live and fighting

if i'm not left alone
maybe...
changes can come more easily

avatar nikosdion
nikosdion - comment - 8 Apr 2020

@alikon If Joomla really wants to fix this issue they can merge @dgrammatiko PR #13499 It is exactly what we discussed that needs to be done. Not that there's a snowball's chance in hell for that happening.

See, @HLeithner was trying to convince us that Joomla follows semantic versioning (it doesn't) and this is a b/c break (it isn't; also notable is that that SemVer does not prohibit adding new features in a major version, it only cares about breaking backwards compatibility but who cares about facts when you can have catchy aphorisms that sounds plausible to the non-technically oriented).

So, clearly, the problem here is political, not technical.

Here's the thing. If we are SERIOUSLY saying that having a feature in core that can EVEN REMOTELY lead to an XSS vulnerability in the core due to POSSIBLY inadequate filtering of scripts we should IMMEDIATELY REMOVE any feature that allows submitting HTML in the frontend because I know FOR A FACT that Joomla's HTML filtering if set to anything other than "No HTML" is not cutting it – and I made long comments about it in this PR so if anyone tells me they didn't know I'll show them a damn liar and a moron! If we really care so much about the possibility of a malicious upload being opened by a Super User we MUST disable ALL uploads from non Super User accounts because a malicious Manager or even Administrator can very plausibly upload a malicious PDF, DOC, XLS or even JPG that has FAR MORE SERIOUS consequences to the victim's device security (as opposed to just hacking a site). But of course we are not doing any of this crazy crap because that would be absurd for a CMS! So why are we not implementing SVG support in Joomla if not for a 100% POLITICAL REASON? The only other explanation would be that someone's ego gets sorely hurt when they find out that the mantra they were blithely repeating for ten years has stopped being relevant for, dunno, 3 to 5 years now? More? This, too, sounds like a political – not a technical – reason to me. But what do I know? I just write code...

As for fighting the good fight from the inside do you know what I got for my trouble? Idiotic remarks on Twitter. I even got blamed because someone else didn't know that closing a PR doesn't make the code go away. Hello? Anybody ever clicked the Changed Files tab on GitHub? How is it my fault that they didn't?! And how THE HELL did they get the impression that I ever said that implementing SVG support is technically challenging when I keep repeating that the solution is straightforward, it just needs a reasonable amount of time to implement (2-3 days) and an unreasonable mount to discuss it to oblivion (20-50 hours)? The technical challenge is BARELY there for anyone who knows how to write code. A feature that goes through 3 revisions (1 line to half an hour to 2-3 days) is NOT challenging. Even a 4-week long feature is not challenging. If you want to talk about challenging code try something that requires a massive refactoring that takes 6 to 18 months just to do the groundwork. I've done a few, I should know.

Nicola, if you want to keep pushing for reason and technical discussion in here feel free. The rest of us are picking up the last few remaining shreds of our sanity and disembarking this crazy train.

avatar HLeithner
HLeithner - comment - 8 Apr 2020

See, @HLeithner was trying to convince us that Joomla follows semantic versioning (it doesn't) and this is a b/c break (it isn't; also notable is that that SemVer does not prohibit adding new features in a major version, it only cares about breaking backwards compatibility but who cares about facts when you can have catchy aphorisms that sounds plausible to the non-technically oriented).

I only try to follow our https://developer.joomla.org/news/586-joomla-development-strategy.html this includes no new features in patch/bugfix releases only in minor or major.

And Dimitris none of your linked PRs has been blocked because you are adding SVG images the first you closed for an unknown reason and the other 2 are had other discussion points and not the fact that you add svg images.

Anyway I get already got enough strokes for adding supposedly features into patch release in 3.9.x and it seams it's already fixed in j4. Beside that we are basically in feature freeze for j3 and supporting svg as the only cms seams to be a major feature.

Beside that we want to get j4 ready and released, having endless discussion about removing the feature freeze for j3 doesn't help much. We are really near on a beta for j4 so we finally want to concentrate on that release so the vaporware sign can be removed.

avatar crystalenka
crystalenka - comment - 8 Apr 2020

It’s really not a feature. It’s a fix. The media manager does not honor the allowed content types in the options as it appears it should. Adding preview, and ability to select, for svgs in joomla media manager is not a feature. As this PR notes, it’s literally one line of code.

Joomla will happily let you upload svgs. Is it not better to let us preview them and select them, instead of haphazardly typing in the link?

If it’s not backwards incompatible - and clearly it’s not - then I really don’t know what the problem is here.

avatar Fedik
Fedik - comment - 8 Apr 2020

This actually a very old bug: not able to see and select the image with specific format(s),
so all fine here ?

All that need is update the model with hardcoded image extensions

upd. I can redo pr, if no one else do, maybe at weekend

avatar coolcat-creations
coolcat-creations - comment - 8 Apr 2020

I actually had the question from customers in the last 2 Months 4 times and I can't count how often before why they don't see the svgs. I think also that it's a bugfix.

avatar mbabker
mbabker - comment - 9 Apr 2020

For the record, I like how the SemVer argument is thrown in people's faces when they're trying to do something meaningful, but when the Production Department decides it to be inconvenient to follow the established development strategy then it is ignored. Hint, deprecations cannot go into a patch release. Just flat out say you don't want Nic contributing and stop beating around the bush.

avatar crystalenka
crystalenka - comment - 9 Apr 2020

The most frustrating part for me, as a site builder, is that instead of official Joomla people acknowledging this as a bug/enhancement or whatever and helping us figure out when it can be included, they are choosing to completely shoot down the idea. It's not constructive, and it makes us feel unheard.

This is a small but real and very frustrating bug for those of us building sites on a daily basis. What needs to happen for us to fix it? Joomla 4 stable, for production sites, is not going to be out soon enough for us to just ignore this.

I really, really don't want to have to core hack every time I build a site and update it. (It's just one line!)

avatar marcodings
marcodings - comment - 9 Apr 2020

I'm not a sure what the alleged "political" reasoning for rejecting the idea even could be. But i would state there are none.

I guess its no secret that personally i'm a fan of using SVG's and i have read some new/updated idea's today.

The topic of supporting SVG's has Production Departments attention is is being actively discussed!

avatar crystalenka
crystalenka - comment - 9 Apr 2020

Our messages crossed. Thanks @marcodings. :) That's really good to hear.

avatar sanek4life
sanek4life - comment - 9 Apr 2020

Maybe there is such a solution: Allow uploading SVG only through the Control Panel, and do not allow uploading SVG through the frontend (frontend editor, frontend comments, frontend chat/messages, etc).

avatar progreccor
progreccor - comment - 9 Apr 2020

Maybe there is such a solution: Allow uploading SVG only through the Control Panel, and do not allow uploading SVG through the front-end (front-end editor, front-end comments, front-end chat/messages, etc).

I think that is the real solution

avatar nikosdion nikosdion - change - 10 Apr 2020
The description was changed
avatar nikosdion nikosdion - edited - 10 Apr 2020
avatar N6REJ
N6REJ - comment - 15 Jun 2020

for us to allow & use svg fonts ( see atum ) and yet not allow it everywhere else is silly.
@nikosdion don't give up nick.

avatar brianteeman
brianteeman - comment - 15 Jun 2020

You are missing the point

avatar nikosdion
nikosdion - comment - 15 Jun 2020

SVG images and SVG fonts are very different things.

avatar N6REJ
N6REJ - comment - 15 Jun 2020

You are missing the point

Actually I'm not.. modern image formats & handling methods, which can have a significant impact on sites, are NOT being used in J!

avatar brianteeman
brianteeman - comment - 15 Jun 2020

Yes they can have a significant impact on sites. They can lead to them being hacked. That's the point.

avatar alikon
alikon - comment - 15 Jun 2020

just discovered that i've not the power to reopen this 1
@joomla/cms-maintainers can you do it this please?

avatar richard67
richard67 - comment - 15 Jun 2020

@alikon I can't either. Seems only the author can do that with a PR, or it is because the branch has been deleted meanwhile, so it shows "unknown repository" as branch.

avatar Fedik
Fedik - comment - 15 Jun 2020

can someone tell what exactly security issue with use of SVG, and in which cases?
for SVG in <img> tag (not for inline SVG, because it is obvious),

because it sounds like:

- you see an alien? 
- no,
- but it there
avatar nikosdion
nikosdion - comment - 16 Jun 2020

@Fedik Follow the links in my reply from early April. I linked to this information so that people who are interested in understanding what is the security context of using SVG files, like you, could follow them.

@richard67 Can't reopen it. Trying to update my Joomla fork I cocked it up so I decided the best way is to delete my forked repo and start afresh. At this point I had no open PRs in Joomla so it didn't sound like a bad idea at all. Well... that branch for this PR? Gone. You can still take a look at my plugin and see how I did it. It simply patches the Joomla core classes in memory.

If you are sure you want me to make a PR I can reverse engineer the patches I am doing in my plugin and convert them into a PR. However, I will only spend this time IF AND ONLY IF I have the JSST come here and explicitly state that they will accept a PR based on the SVG sanitization I am doing in the plugin I linked to in my PR's comment. If they don't explicitly say that then I won't waste my time and you can still use my plugin.

avatar Fedik
Fedik - comment - 16 Jun 2020

@nikosdion thanks for link, I just wanted to write that there no issue with <img>, but you explained it much better.
Only issue if someone open the file directly.

@SniperSister we do not need to filter whole SVG, or do any content manipulation, we already have a fancy method that after some improvement can be much useful, InputFilter::isSafeFile()

public static function isSafeFile($file, $options = array())

That used in File::upload()
$isSafe = \JFilterInput::isSafeFile($descriptor, $safeFileOptions);

All we need is to check whether SVG contain <script> tag and stop upload if any.

avatar nikosdion
nikosdion - comment - 16 Jun 2020

@Fedik We already discussed this. It is possible to have attributes with inline JavaScript such as onclick. Using sanitization addresses this since only explicitly allowed elements and attributes will make it into the resulting SVG.

The problem is that the project thinks this is a “major change” and won't include it in Joomla 3. At the same time, Joomla 3.10 backports features from Joomla 4 which have a far more serious and far reaching impact.

The real problem that nobody admits is who takes responsibility for the code. I am ready to do so, as I've always done for the code I contributed to the project. The thing is, the leadership doesn't want to take that tiny sliver of responsibility for letting me contribute my code. So we are at a stalemate and I created my own plugin to manage SVGs in my own sites.

avatar coolcat-creations
coolcat-creations - comment - 16 Jun 2020

@marcodings can this be made possible? In my personal opinion it's a very important bugfix / function.

avatar SniperSister
SniperSister - comment - 16 Jun 2020

However, I will only spend this time IF AND ONLY IF I have the JSST come here and explicitly state that they will accept a PR based on the SVG sanitization I am doing in the plugin I linked to in my PR's comment. If they don't explicitly say that then I won't waste my time and you can still use my plugin.

Having that topic on the agenda for today's JSST meeting

avatar richard67
richard67 - comment - 16 Jun 2020

Beside the technical aspects I want to say thank you to @nikosdion that despite of this long and frustrating discussion he has not given up and is still contributing to the project and still is willing to work on this issue. This shows your love to the project, @nikosdion , thank you very much for that.

avatar Fedik
Fedik - comment - 16 Jun 2020

We already discussed this. It is possible to have attributes with inline JavaScript such as onclick. Using sanitization addresses this since only explicitly allowed elements and attributes will make it into the resulting SVG.

if ('onclick' in $svgContent or 'script' in $svgContent): 
  then stop upload

(and some other events attributes)

easy ?

The real problem that nobody admits is who takes responsibility for the code.

I understood

avatar nikosdion
nikosdion - comment - 16 Jun 2020

@Fedik Unfortunately it's not that easy :( You really need to go through the DOM to sanitize the file based on an allow list of tags and attributes. The problem is that browsers use their very lax and forgiving SGML parser even on what is ostensibly a strict XML file type like SVG. There are various crap a malicious actor can do to obscure the fact they are using an attribute and the browser still parse it. Your only real defense is full parsing, sanitization and writing out the sanitized file.

avatar coolcat-creations
coolcat-creations - comment - 16 Jun 2020

Maybe a stupid question but I could also upload a malicious php file into template when I am a superadmin or not? Where is this security risk different from uploading an insecure svg?

avatar brianteeman
brianteeman - comment - 16 Jun 2020

@coolcat-creations yes you could which is why the default access to do that is super admin. The equivalent would be making the default access for media manager is also super admin

avatar nikosdion
nikosdion - comment - 16 Jun 2020

@coolcat-creations Besides what @brianteeman already said, do keep in mind that the media manager settings apply to all uploads across all components in Joomla. Even if Media Manager was Super User only you could still upload a malicious SVG through a WYSIWYG editor, a forum component, a support tickets component etc. So what we do in that little configuration page has an impact throughout the Joomla ecosystem. That's why I first closed this PR in favor of going to a full implementation of SVG sanitization.

avatar coolcat-creations
coolcat-creations - comment - 16 Jun 2020

Thank you both for explaining. I am looking forward to use svgs in Joomla without any hassle. Til date I upload them by ftp and copy paste the paths which is just not nice for the customers to handle. They often delete the "ghostpath" by accident and can't select it back...

avatar b2z
b2z - comment - 16 Jun 2020

Thank you both for explaining. I am looking forward to use svgs in Joomla without any hassle. Til date I upload them by ftp and copy paste the paths which is just not nice for the customers to handle. They often delete the "ghostpath" by accident and can't select it back...

Just install alternative media manager, like Quantum, for example :)

avatar SniperSister
SniperSister - comment - 16 Jun 2020

However, I will only spend this time IF AND ONLY IF I have the JSST come here and explicitly state that they will accept a PR based on the SVG sanitization I am doing in the plugin I linked to in my PR's comment. If they don't explicitly say that then I won't waste my time and you can still use my plugin.

@nikosdion issue was discussed during the meeting and proposal for "min requirements for SVG support" are up for comment for the other team members. Will share the outcome on thursday!

avatar nikosdion
nikosdion - comment - 16 Jun 2020

@SniperSister That's perfect! Thank you. Just a heads up, I will be mostly unavailable between Thursday afternoon and Friday afternoon. I will try to respond as soon as I can when I receive your feedback. Have a great evening!

avatar SniperSister
SniperSister - comment - 18 Jun 2020

@nikosdion as promised, here's the feedback from the meeting:

The JSST will accept the addition of SVG support to the CMS, if two requirements are met:

  • a SVG sanitization library is used (like in your plugin)
  • SVG upload for non super-admins is disabled by default
avatar nikosdion
nikosdion - comment - 18 Jun 2020

Since upload restrictions are global and not per user level what you ask is simply not possible. It is also utter nonsense since a Super User can already install a file manager and circumvent Joomla’s sanitization for SVG files.

I would like to once again underscore the baldfaced hypocrisy of your decision. Joomla 4 allows for unsanitized SVG uploads from everybody.

People, if you want SVGs in Joomla 3 either use my plugin or JCE Pro. I give up.

avatar zero-24
zero-24 - comment - 18 Jun 2020

Since upload restrictions are global and not per user level what you ask is simply not possible. It is also utter nonsense since a Super User can already install a file manager and circumvent Joomla’s sanitization for SVG files.

Well the aim is just to have it disabled by default?

I would like to once again underscore the baldfaced hypocrisy of your decision. Joomla 4 allows for unsanitized SVG uploads from everybody.

I have not checked that myself but george told us 4.x only shows uploaded SVG's but do not allow to upload them.

People, if you want SVGs in Joomla 3 either use my plugin or JCE Pro. I give up.

That would be the case for 3.x anyway. I personally would add that SVG cleanup and upload feature to 4.x only.

avatar SniperSister
SniperSister - comment - 18 Jun 2020

Since upload restrictions are global and not per user level what you ask is simply not possible. It is also utter nonsense since a Super User can already install a file manager and circumvent Joomla’s sanitization for SVG files.

Sorry, that's unclear in my first statement:
SVG upload should be disabled by default, turning it on should be a super-admin setting (like it is today). So, no need for per-user-level restrictions and no blocker from our side.

avatar richard67
richard67 - comment - 18 Jun 2020

That's more clear. @nikosdion Is this ok for you?

avatar nikosdion
nikosdion - comment - 18 Jun 2020

@SniperSister This is completely different to what you wrote. Let me explain what I can do (and why).

I am NOT touching the extensions and MIME types allowed. As a result, SVG upload is disabled by default. A Super User needs to go in there and change them. So, by default, there is no SVG support unless you are BOTH a Super User AND know what the heck you're doing. I think that is more than the minimum bar you set.

Change MediaHelper and BannerHelper to respect the Media Manager's settings about which extensions are considered images. This creates feature parity with Joomla 4 and is reasonable – as far as users are concerned, the image types option currently "does nothing" which is bad UX. Moreover, it allows for the display of SVGs as images and picking them in an image picker IF AND ONLY IF a Super User explicitly adds the SVG extension as a valid image extension.

When uploading files, if the file has an svg extension (case insensitive) it will go through the SVG sanitizer library. If the sanitizer fails to sanitize the file we throw a RuntimeException which forces the upload the fail with a message that this is not a valid SVG file.

Would that be OK?

avatar brianteeman
brianteeman - comment - 18 Jun 2020

Can you clarify if we are talking about J3, J4 or both

avatar SniperSister
SniperSister - comment - 18 Jun 2020

This is completely different to what you wrote. Let me explain what I can do (and why).

Sorry, it has been a loooong day ;)

Would that be OK?

Yes, perfectly fine for me.

Can you clarify if we are talking about J3, J4 or both

The JSST statement is for both, J3 and J4. We have no security-related objections against an implementation in any of those two versions.

If a J3 PR is accepted by the maintainers (not counting me in here as I honestly neither deserve nor fulfill that role) under semver-aspects is a question that I can't answer.

My very personal opinion as a Joomla user is that SVG supports adds so much additional value to 3.x, that it should be added to it, especially having the two more years of support in mind.

avatar robertnormanhiggins
robertnormanhiggins - comment - 19 Jun 2020

I got here because my J4 beta template styles editor does not even show the logo.svg from templates/cassiopeia/images. I'm trying to assess all the issues of svg presented here and various related settings. But I am surprised that the default template has svg files that cannot by default be edited/changed in the template styles editor.

avatar ceford
ceford - comment - 1 Dec 2020

What happened to this? We currently have svg upload optional but this bit of code on line 300 of MediaHelper.php (in J4):

$xss_check = file_get_contents($file['tmp_name'], false, null, -1, 256);

ensures that tests against all of the html_tags tags will never find any. So no sanitization and no check for bad svg content.

Add a Comment

Login with GitHub to post a comment