Last modified: 2014-10-15 07:59:11 UTC
when adding a link to an external image from a domain that is allowed ( via http://www.mediawiki.org/wiki/Manual:$wgAllowExternalImages or http://www.mediawiki.org/wiki/Manual:$wgAllowExternalImagesFrom or http://www.mediawiki.org/wiki/Manual:$wgEnableImageWhitelist ), files with extension gif|png|jpg|jpeg result in the image being inlined, but this is not the case for SVG files. The following fixes this: in \includes\parser\Parser.php, line 91, 92: const EXT_IMAGE_REGEX = '/^(http:\/\/|https:\/\/)([^][<>"\\x00-\\x20\\x7F\p{Zs}]+) \\/([A-Za-z0-9_.,~%\\-+&;#*?!=()@\\x80-\\xFF]+)\\.((?i)gif|png|jpg|jpeg)$/Sxu'; change that to : const EXT_IMAGE_REGEX = '/^(http:\/\/|https:\/\/)([^][<>"\\x00-\\x20\\x7F\p{Zs}]+) \\/([A-Za-z0-9_.,~%\\-+&;#*?!=()@\\x80-\\xFF]+)\\.((?i)gif|png|jpg|jpeg|svg)$/Sxu'; Note the simple addition of svg. I have done so myself in mediawiki 1.22.5, and this allows SVGs to be visible on the page, in Chrome 33.0.1750.154 m
Hi! Thanks for the report and investigating a fix! You are welcome to use Developer access https://www.mediawiki.org/wiki/Developer_access to submit the code change as a Git branch directly into Gerrit: https://www.mediawiki.org/wiki/Git/Tutorial If you don't want to set up Git/Gerrit, you can also use https://tools.wmflabs.org/gerrit-patch-uploader/
Change 139680 had a related patch set uploaded by ImPacific: fix bug 63806 (svg external image placed) https://gerrit.wikimedia.org/r/139680
Change 139680 had a related patch set uploaded by TheDJ: Recognize SVGs for External Images https://gerrit.wikimedia.org/r/139680
Copy-and-paste some discussion from IRC: cscott-free: gwicke: https://bugzilla.wikimedia.org/show_bug.cgi?id=63806 allows $wgAllowExternalImages to include .svgs as well as png/jpg/gif/etc. cscott-free: gwicke: the patch itself looks trivial. i'm a little concerned about the possible security implications... but presumably added an appropriate RELEASE NOTES entry would be sufficient. cscott-free: gwicke: if you've got some extra cycles to double check that and make sure i'm not missing any obvious ops/security problems i'd appreciate it. gwicke: I'm not terribly familiar with SVG security issues gwicke: not sure if SVGs can embed JS for example cscott-free: i believe that they can cscott-free: further, they can use external URLs for resources, which is enough to do port scanning, etc gwicke: that could be an invitation to steal cookies by external-linking to a prepared SVG image original hosted on the same wiki cscott-free: right. but the owner of the wiki presumably has added an explicit whitelist, so trusts the site. cscott-free: which mitigates the threat to some degree. cscott-free: the only new security issue is that the owner of the wiki might have some vulnerable svgs sitting around, which they didn't audit because they weren't previously linkable. or something like that. cscott-free: but if you've enabled $wgAllowExternalImages you're asking for trouble. gwicke: normally that's fairly harmless gwicke: it's different with SVGs though gwicke: could be at least cscott-free: yeah. what's your intuition there? just release-note it? or is this different-enough trouble that there should be an explicit $wgAllowExternalImagesYesSVGsToo opt-in? gwicke: so SVG does support onmouseover etc cscott-free: yeah, SVG supports full HTML embedding as well, although i don't think support is present in all browsers. so basically anything you can do in raw HTML embedded in the page you can do in an embedded SVG. gwicke: I don't think that just adding them without investigating the security implications more deeply would be a good idea cscott-free: i'm going to suggest to the author that they add an opt-in variable, then. cscott-free: it makes the patch more complicated than just adding '|svg' to a regexp, but better safe than sorry. gwicke: yeah, that sounds like a better approach cscott-free: thanks To recap: we think that, because there are security implications, we shouldn't allow .svgs as external images unless the wiki owner has explicitly set a configuration variable to opt-in to this behavior. The new variable should also be mentioned in the RELEASE NOTES. Hopefully these changes won't be too difficult to implement. Thanks for your contribution so far!
(In reply to C. Scott Ananian from comment #4) > To recap: we think that, because there are security implications, we > shouldn't allow .svgs as external images unless the wiki owner has > explicitly set a configuration variable to opt-in to this behavior. > > The new variable should also be mentioned in the RELEASE NOTES. My initial thought is to agree with gwicke, and this should have it's own flag. The SOP of javascript in svg files is a little murky, and each browser has their own way of implementing controls around it. I'd rather be safe and realize a year from now we can combine the flags than suddenly put everyone using the existing functionality at risk.
thanks all for the feedback here and at gerrit. I was not aware of the potential risks involved with SVGs. I Will try my hand at implementing the configuration variable if possible :)
