@test
Although I can now insert an SVG image indeed, I still can't upload any through media manager
(added image/svg mime type, and legal extension)
I still get "Not a valid image."

@test
like jean marie this is not enough as it still doesnt allow you to upload an svg.

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

Hello,

that was on purpose, because I don't like patches with several new stuff to test. So the plan was:

• first add the possibility to choose SVG files in the image manager
• if accepted, I wanted to create a new patch which extends the upload for uploading SVG files (which is a bigger code change than this patch)
  
  This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4674.

Well we cant have one patch without the other and it is harder for people to text a patch if it relies on another one so it would be better if you create just one patch

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

Hello Brian,

I see your point and in general I would definitely agree with you. But in this case (I think) it's not a good idea.
The main reason is, that Joomla! has a XSS filter for uploaded files, which blocks inline code (like <!DOCTYPE, <!--, and (perhaps) other relevant stuff for SVG). So we need a bigger rewrite of the upload handling and we have to be careful, when we change stuff there. So in my opinion, I still think, that we should separated the two patches (they depend only indirect) and you have to upload the SVG files via FTP client for testing.

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

Yes but the problem is that their is zero point in committing this pull
request on its own as it will just lead to confusion

On 15 October 2014 12:59, bembelimen notifications@github.com wrote:

Hello Brian,

I see your point and in general I would definitely agree with you. But in
this case (I think) it's not a good idea.
The main reason is, that Joomla! has a XSS filter for uploaded files,
which blocks inline code (like <!DOCTYPE, <!--, and (perhaps) other
relevant stuff for SVG). So we need a bigger rewrite of the upload handling
and we have to be careful, when we change stuff there. So in my opinion, I
still think, that we should separated the two patches (they depend only
indirect) and you have to upload the SVG files via FTP client for testing.

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/4674
http://issues.joomla.org/tracker/joomla-cms/4674.

—
Reply to this email directly or view it on GitHub
#4674 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

I still see a point in commiting this patch, because I can offer my authors a folder of SVG files which they can use (no upload required).

To infograf768: SVG files are no images, they are "normal" files. So they cannot be validated as images (you have to add the extension to the "Legal Extensions (File Types)" parameter).

Depending on the SVG structur you'll trigger the XSS check in Joomla! or not. So in general you can upload SVG files without a new patch, if they have a structur, Joomla! likes ;) (no <!DOCTYPE in the first 265 chars e.g.)

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

for this PR. Reason is that in this case we call getimagesize on the image. This is a really bad idea as it returns false for any SVG's

This is also the reason why you can't upload them in media manager - this line always returns false for an svg https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/helper/media.php#L139 so you get the error brian quoted.

If we want to support SVG's in media manager (and with my worker for Virya Group hat on I really REALLY do). We're gonna have to rewrite some parts of media manager to reduce our dependency on getimagesize because all whilst we use that support for SVG's is gonna be bad-none

I just can repeat myself:
It is possible to upload SVG files, just allow them in the options under "Legal Extensions (File Types)" and (if you have activated the mime-check) add it to the mime list. Due the fact, that SVG are no image files but vector graphics (XML based) it's not necessary to add "svg" to the "Legal Image Extensions (File Types)" option. So the only thing which is missing is the possibility to choose them after uploading in the media manager when choosing a image. => so when we have applied this patch we can now not only upload SVG files (which is already implemented) but also choose them.

Some stuff you could insert into a SVG file will trigger the XSS check (1). So if someone needs better support for SVG files while uploading them, then he/she/it could improve the handling with a new patch...but this does not associated with this patch, because it just adds basic support for choosing.

(1) Just make sure, that the first 256 characters does not contain invalid strings


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

cant upload *.svg file

@symatic thanks for testing. this PR only adds the ability to see them in the list and select them - it does not allow you to upload them. Weird I know but thats what @bembelimen wanted to do

@test
When trying to upload in media manager after applying the patch and adding svg to legal extensions I get this error:
"Notice: Possible IE XSS Attack found."

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

Without patch, it's the same error. Tried several svg images.

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

As said multiple times above this is not to enable image uploads but just
to enable the ability to see SVG files in the image list. Not my idea to
do it this way ;(

On 17 October 2014 13:44, Ralf Longwitz notifications@github.com wrote:

Without patch, it's the same error. Tried several svg images.

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/4674
http://issues.joomla.org/tracker/joomla-cms/4674.

—
Reply to this email directly or view it on GitHub
#4674 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Yep, that's a "bug" in the XSS check.

Try this file (unzip first): http://stuff.benjamintrenkle.de/address-book.zip

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

hmm apparently forget to commit my answer yesterday

@bembelimen thanks for this PR, svgs are and will become more important imho

1) you are adding svg, but you probaly should add svgz as they both are typically listed as part of the mime type

2) as for the last post, uploading svg theoretically should work. In practice i have not been able to succeed with a wide range of files, most w3c validated.
Correctly configured i get "Possible IE XSS Attack found." ( instead of "This file type is not supported." ).

I'll look into surpresssing the XSS. Also in relation to the new media manager due for 3.5*+? #3839

@brianteeman Alright, sorry. Still, this is not the kind of error message I'd expect.
@bembelimen Could you please add clearer testing instructions?

@marcodings you cant supress the error message - there is a potential
security issue (not for this public list)

On 17 October 2014 14:30, Ralf Longwitz notifications@github.com wrote:

@brianteeman https://github.com/brianteeman Alright, sorry. Still, this
is not the kind of error message I'd expect.
@bembelimen https://github.com/bembelimen Could you please add clearer
testing instructions?

—
Reply to this email directly or view it on GitHub
#4674 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@brianteeman i would not even consider removing it bluntly, but looking into how to selectavely surpress / replace it with an alternative test specifically for svg.

(contact me off list please)

On 17 October 2014 14:35, marcodings notifications@github.com wrote:

@brianteeman https://github.com/brianteeman i would not even consider
removing it bluntly, but looking into how to selectavely surpress / replace
it with an alternative test specifically for svg.

—
Reply to this email directly or view it on GitHub
#4674 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

OK on the basis that yes the PR does what it says "adds svg to the list" and nothing more than that I am going to change my test to a success. I still have "issues" if this is worth committing as it is

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

I am setting this to Needs Review so that the maintainers can make a decision. The PR does what it says but it might lead to confusion as it does not provide full support for SVG

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

so what?
2015 on the yard and still can't upload svg file with media-manager...

The problem with SVG's is that by definition of how they function you can upload malicious code as part of them - see http://blog.guya.net/2014/02/17/svg-for-fun-and-phishing/ for example. This means anyone who can upload regular images can theoretically upload malicious code. This makes it really hard to find a valid solution that works. The company I work for do a huge amount of work with SVG's so believe me I want them to be supported in core more than anyone else in this thread!

Okay... so I understand the big risk of embedded malware inside svg-files.
Would it be an option to use ACL as some kind of layer of security? As in: "only a given group of users can upload svg files"

That's a good suggestion, as SVGs seem to be used mostly for design/styling purposes.

@bembelimen The PLT has decided that this PR is not going to be included as-is. The support for SVG's should be complete including uploading and security check on it's contents.

I am going to close this PR and a new one can be made with complete SVG support. Thank you for your contribution.

That's annoying, some support is better than no support. Is there a workaround to use SVGs? Or do I have to hardcode them or create my own module?

For others reading this, and perhaps for others who develop Joomla! and feel this is a huge security hole, you can use SVGs in Joomla! in two ways.

(a) you can type in the SVG code, which is long winded and messy for clients. It also wouldn't work for an intro image.
(b) you can save your SVG and manually type in the link location in the image window.

Hurruh.

@uglyeoin I agree with your options to insert the svg, but getimagesize is still an issue. The preview will not show as a result in the blog view of a category. Maybe additional fields to add the size would work when entering the URL to the image?

I guess the point is, you can still use SVGs, so although seen as a security hole in Joomla, it's already possible to use them. I'd also suggest that if there is a fear of uploading malicious code, it's the users responsibility. Surely Joomla! do not want to take responsibility for this? If taking ownership, then perhaps installation of components should be regulated. Perhaps automatic ban of any items on the unsafe list.

@uglyeoin I ran across this thread and was hoping someone had found a workaround. Maybe someone will figure something out and post it. If we were given the option to manually add the size, that would help.

I'm just going to leave this here regarding why it's such a PITA to properly (and securely) support SVG. https://hackerone.com/reports/148853

Excellent. So are you suggesting we should stop users from being able to use them in Joomla? Because at the moment I can just link to an SVG as I would any external image (even if stored on my site). So Joomla! doesn't prevent me using SVGs, it just makes it annoying.

In any case, it looks like they solved the problem, so perhaps we could ask Abdullah to help us resolve any issues at our end.

In that specific platform's case their fix was to force SVG files to be sent with a plain text MIME instead of whatever it would be normally which allows the XSS bug reported via that issue to be executed.

I'm not trying to say Joomla shouldn't support SVG, but implementing support means that the platform should NOT be opening security holes and if those can't be mitigated then it shouldn't be supported. And from a security perspective the SVG format comes with several issues which is why it's basically been on a blacklist in Joomla's code for years.

I understand, it's just that we neither support it nor ban it. So we haven't black listed it very well at all.

Without adding additional configuration values to the htaccess.txt and web.config.txt files Joomla doesn't go beyond blocks in the Media Manager to implement a full ban, and IMO by default I don't think Joomla has any reason to be messing with how stuff in your filesystem outside the application gets loaded. And since Joomla doesn't try to serve stuff in the filesystem unless you have a component doing so, no PHP code could do anything about it. It's one of those things where if you know your way around you can make it work but it's not user friendly by any means.

we are actively looking into supporting svg to the level that is reasonable and possibly sanitise or at least warn in the new media manager.

Upload support is however very likely to restricted to backend and accesslevels. As the risks are to high to open this to the world and thus open the floodgates. It will come with a lot warnings.

This is prelimenary and work in progress..

Thanks for the update @marcodings , thanks for your insights too @mbabker it helps us little guys to see where bottle necks are and what needs to be changed and potentially opens our eyes to ways we could try to help, even if that is merely spreading this information so others are less frustrated.