Page MenuHomePhabricator

Investigate SVG's uploaded with external resources
Closed, ResolvedPublic

Description

On bug 65724 #c1, Christian linked to an uploaded SVG that included a png loaded via https from upload.wikimedia.org. I confirmed in firefox that the browser is making a separate http connection to load the resource.

If an editor can embed a raw svg in a page, and if the svg loads resources from other web servers, it might be possible to track viewers.

Additionally, if it can load an svg that contains javascript, I need to test what SoP is applied to the javascript, to make sure it can't talk back to our sites.


Version: 1.24rc
Severity: normal
See Also:
http://bugzilla.wikimedia.org/show_bug.cgi?id=3537

Details

Reference
bz65839

Event Timeline

bzimport raised the priority of this task from to Normal.
bzimport set Reference to bz65839.
csteipp created this task.May 28 2014, 12:06 AM

It looks like Chrome and Firefox do the sane thing and don't execute javascript from an image included via xlink. Opera 12 does execute it in some cases.

Javascript SoP is the included domain, so a wiki image that includes an svg from another domain won't have javascript access to the wiki.

The image is able to be uploaded because I check the namespace of elements against our whitelist, but not attributes. I have a patch to also whitelist the attributes, but still testing it.

Created attachment 15514
Forbid remote href's except for <a> elements

The only thing in my corpus of commons SVG's that this now prevents that we might want to allow is <color-profile> elements with an href attribute. All of the examples I found were to 'C:\...' urls, so it may not be a bad thing to deny them just to protect the privacy of our uploader's filesystem.

Also, lots of images had <a> elements pointing to WMF wikis and other websites. This means a click on the SVG could take the user to another website. If we want to start forbidding that, it's easy to update the patch.

attachment bug65839.patch ignored as obsolete

Bawolff / Gergő,

Adding you guys because I wanted to get an opinion on if this is an OK technical change for the commons community.

It seems like at least Christian on bug 65724 figured out one way to embed external images, even if they didn't show up in the thumbnail. But is that something commonly done? When I tested this patch against 25,000 commons SVG's, it found a handful (30 or so) of SVG's with embedded images, over half of those were to local filesystems or relative url paths that just wouldn't work on our servers.

But let me know if you think the community is going to be really disappointed with this, or if you know of anyone else who should give input.

My phone is being stupid and i cant view the patch right now. As long as we still allow data urls, i dont see it being an issue. However i dont generally create svgs for commons, so if we want to be sure we might want to ask a prominent svg author in our community.

Tgr added a comment.May 30 2014, 12:09 AM

Based on the usage of {{BadSVG}} [1] there are about 1000 SVGs with raster images - most use data: URIs though, and as far as I can see that is always an option to anyone intending to embed bitmaps.

[1] https://tools.wmflabs.org/templatecount/?lang=commons&name=BadSVG&namespace=10

Adding NordNordWest and Perhelion on Keegan's recommendation, as users who know how SVGs are used on commons.

NordNordWest / Perhelion, I'm considering deploying this fix on commons to protect our users privacy when they view SVG images in their browser. At this time, since SVGs are always displayed as png thumbnails, a user can't upload an svg that pings their own server and get a report of every IP that viewed the image, but it would become possible if we ever move to showing the actual SVG.

Since thumbnails of SVG with external resources are broken currently (bug 65724), and users always get thumbnails for SVG's at this time, it doesn't seem like this change will break any existing functionality. However, I wanted to check with you to make sure I'm not missing a significant use case where SVG authors embed external images.

As noted on wikitech-l (http://lists.wikimedia.org/pipermail/wikitech-l/2014-May/076691.html), there's a chance we might allow embedding external images from WMF domains, although doing that safely will take a fair amount of work. If that happens, we would make an exception in this upload filter as well.

I asked Isarra, and she didnt think it would be a problem

I'm into maps and I can think of a lot of reasons why it would be great to use an external image, e.g. you have to change only one file and lots of maps are updated. But I don't think that this should be allowed, not even with files which were uploaded at WMF domains. First of all – in my opinion – a license is given for all contents of a file. With external files it might become quite complicated with licenses and naming authors, especially for re-users. And if an external file gets deleted or violated all files which use this file get violated, too. I see more problems than advantages. If it will get possible to use external files in SVGs then at least there has to be a notification to all users who uploaded SVGs with this file when it get changed. Otherwise there is no control what their SVGs show.

Yes I also think this would be a nice (optimization) "feature" and I would see this only most advantage in maps. But I think NordNordWest is also be right there and the (not technical) problems are to big to be solved yet simply.

The simplest way (I guess) would be a clear declared information in both file description pages (per extra template).

All in all this need some more extra tasks for all.

Another point could be the (extra) traffic for Wikimedia, because all copies in the world need to be online and always link there? (I'm not so aware in this)

Anyway I would support the permission of Mediawiki urls.

Anyway SVG is optimation, Mediawiki stays for SVG, Mediawiki is "open" and "free", some people are still against SVG, let us show a bit more of the real potential of SVG! This is a very nice part of SVG, we should use it, most people are first most sceptic on new things, let us do it!

As we can see the people use image links (ok most as local link) in SVG, but I guess most of them are from Commons (I sometimes see it), so now we can easy fix them. I see no problem with author or license (or re-use). It would be the same as without image link. Let us tell it to the community.

Is an external file (legal) part of a SVG or not? For me any file is a unit. This ends with external files. Before opening a whole new way to create SVGs and re-use other files it is essential to clear all questions about licenses, authorship, responsibility and control. It is impossible to see what happens when I upload a new version of a file which is linked in SVGs because there will be no file usage list. The other way round (no control when the linked file gets changed) the same problem as I mentioned above. I am not convinced that this is a good idea.

I think you misunderstand:

Current situation: you can upload a file that references an external file. However it wont render that external file. (It will work if you view the original svg in your browser)

Proposed system: files with external embedded files are banned. If you try to upload one you get an error

The allowing external files from wikimedia servers is a separate thing that has been proposed. It probably wont be done without discussion, and definitely not "soon". Its separate from the current discussion

Ok, if this is the question, then block it, as Chris said. And so also the user knows directly why it not works and goes not simply away.

And sorry for the misunderstand. I suggest also a link to the Commons workshop (https://commons.wikimedia.org/wiki/Commons:Graphic_Lab/Illustration_workshop), because some user don't know how this can be happen or fixed.

... or another helpful link.

Block it, especially to end the possible tracking.

Created attachment 15553
Forbid remote href's except for <a> elements

Updated patch to remove changes to the namespaces. We should do those as a public change, and not make the security patch any bigger than necessary (as pointed out by Brad via irc)

Attached:

PRO and NordNordWest, thank you very much for the comments! We'll keep investigating the legal ramifications of including other images hosted on WMF wikis, but until then, we'll just block them all.

Patch is deployed now. I'll do a public cleanup patch after this is released to give a better error messages to point to some tutorials on SVG files, and to add in the namespaces I found in my testing.

18:58 logmsgbot: csteipp Synchronized php-1.24wmf6/includes/upload/UploadBase.php: (no message) (duration: 00m 04s)
18:51 logmsgbot: csteipp Synchronized php-1.24wmf7/includes/upload/UploadBase.php: (no message) (duration: 00m 06s)

Markus, this can be released with the next tarball.

Created attachment 15708
Backport for REL1_23 branch

Tested with https://commons.wikimedia.org/wiki/File:Bahnstrecke_Zeitz-Camburg_-_Verlauf_um_1930_-_Umnutzung_als_Radweg_ab_Juli_2012_-_hinterlegt_mit_MTB_Detailkarten.svg, which is now rejected. The test file Toll_Texas_1.svg still can be uploaded.

I'd be happy if someone could "+2".

Attached:

Created attachment 15709
Backport for REL1_22 branch

Tested with https://commons.wikimedia.org/wiki/File:Bahnstrecke_Zeitz-Camburg_-_Verlauf_um_1930_-_Umnutzung_als_Radweg_ab_Juli_2012_-_hinterlegt_mit_MTB_Detailkarten.svg, which is now rejected. The test file Toll_Texas_1.svg still can be uploaded.

I'd be happy if someone could "+2".

Attached:

Created attachment 15710
Backport for REL1_21 branch

Tested with https://commons.wikimedia.org/wiki/File:Bahnstrecke_Zeitz-Camburg_-_Verlauf_um_1930_-_Umnutzung_als_Radweg_ab_Juli_2012_-_hinterlegt_mit_MTB_Detailkarten.svg, which is now rejected. The test file Toll_Texas_1.svg still can be uploaded.

I'd be happy if someone could "+2".

Attached:

Created attachment 15711
Backport for REL1_19 branch

Tested with https://commons.wikimedia.org/wiki/File:Bahnstrecke_Zeitz-Camburg_-_Verlauf_um_1930_-_Umnutzung_als_Radweg_ab_Juli_2012_-_hinterlegt_mit_MTB_Detailkarten.svg, which is now rejected. The test file Toll_Texas_1.svg still can be uploaded.

I'd be happy if someone could "+2".

Attached:

Can't upload because a SVG filter is present

I got an ERROR unspecific warning: "This file contains HTML or script code that may be erroneously interpreted by a web browser."
I tested something and is was a simple filter attribute presentation which seems only accepted in a style attribute.

Here the same fixed file: [[File:Topographic map of Cape Verde-de.svg]]

attachment Topographic_map_of_Cape_Verde-de3_cleaned.svg ignored as obsolete

Comment on attachment 15715
Can't upload because a SVG filter is present

Please remove, I opened a new bug 67044.

Giving Wikia early access

Comment on attachment 15715
Can't upload because a SVG filter is present

Removing this since PRO opened another bug

Can we publish this bug? And is there a CVE? People keep asking me about this.

No CVE, I'll update with one if mitre assigns one.

I actually argued against it for this bug (http://seclists.org/oss-sec/2014/q2/646), since I haven't been able to find a way to actually exploit it, so I think this falls into good hardening.

There are two threats:

  • Tracking a user who views a page. Right now, MediaWiki and WMF extensions only put the PNG in the page. If a gadget, or another extension puts the actual SVG into the article, this would be exploitable. I haven't found one that does that, but I wouldn't be surprised if one existed, and there is certainly interest in putting this feature into MediaWiki in the near future.
  • Running javascript in embeded svg files from other domains. As I mentioned, Opera will run the javascript, but the javascript can only talk back to the including domain. If another browser does this wrong, it would open up xss on the wiki, but I haven't found one yet.

But there's also a thread on oss-sec right now about when hardening issues should get a CVE, so based on the outcome of that, this might get one assigned.

csteipp closed this task as Resolved.Apr 13 2015, 7:48 PM