Page MenuHomePhabricator

SVGs fail to render silently if they contain an <image /> element
Closed, ResolvedPublic

Description

Author: ejsanders

Description:
Obviously the <image /> element is disabled, but it should be stripped, or at
least alert the user to the problem, as many users don't know why their SVGs
aren't rendering.


Version: unspecified
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=65839

Details

Reference
bz3537

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 8:52 PM
bzimport set Reference to bz3537.
bzimport added a subscriber: Unknown Object (MLST).

<image> is supported, but only for embedded data: urls.

ejsanders wrote:

I don't understand:
http://en.wikipedia.org/wiki/Image:Map_of_USA_with_state_names.svg failed to
render the first version, the new version works and the only change was the that
I removed the <image /> element.

Let's look at that <image>:

<image
   xlink:href="USA_highway_map.png"
   sodipodi:absref="/home/eric/wiki/USA/USA_highway_map.png"
   width="893.71935927"
   height="568.20873295"
   id="image960"
   x="27.34392929"
   y="150.91799927"
   sodipodi:insensitive="1" />

Notice how the url it uses to reference the image resource is a
relative path, *not* an embedded data: url. It refers to an external
file, which:

  1. Is not allowed by our security policy on the renderer.
  2. Wouldn't work anyway since the referenced file wouldn't be there.

ejsanders wrote:

The bug isn't that the <image> doesn't work, but that it fails silently. It
should either strip disallowed image elements then render, or alert the user to
the violation of the security policy, so they can fix the image.

Yes, that's why the bug hasn't been closed. :)

You asked why a particular image failed.

ejsanders wrote:

(In reply to comment #5)

Yes, that's why the bug hasn't been closed. :)

You asked why a particular image failed.

Ah, I was just giving it as an example, I though you were disagreeing with me
with your first comment.

I agree that there should be an error message on the description page. Or, even
better, this should be detected on upload, and the image rejected.

codedread wrote:

Just to confirm:

  1. data: URLs _ARE_ allowed and supported in the svg image uploads + rasterization to PNGs that happen on wikimedia?
  1. do you scrub data: URLs to ensure that someone hasn't injected another SVG image (with potentially malicious script)?

If the answer is yes to both the above questions, then a potential solution is to update Inkscape so that upon importing a raster image, the user is presented with 3 options: a) embed raster as base64 data:, b) reference local file, c) reference a foreign URL - with sufficient explanations for these three options that a newbie can understand (i.e. "choose option a if you are uploading this file to wikipedia or openclipart")

If I get some feedback here I can open a bug and help push it along for Inkscape (if it hasn't been opened already - if there is an open bug, please link to it here).

Regards,
Jeff Schiller

codedread wrote:

FYI, I have finally opened <a href="https://bugs.launchpad.net/inkscape/+bug/366993">Bug 366993</a> against Inkscape for this issue.

Assigning SVG bugs to Ariel -- need a cleanup pass to see what's fixed up by a librsvg upgrade, what can be resolved with fixes to our font configuration, what can be fixed on our end, and what still needs to be pushed upstream.

Say, I can't actually look at the launchpad bug to see what's up, it's marked private... Jeff, can you do anything about that?

Data URIs _should_ currently work fine; at the moment we implement the restriction against external images by patching librsvg so it literally can't load external resources.

To generalize it via an upload-time check we would indeed want to be able to decode sub-SVG images to check for additional external resources in them; that's a detail I hadn't thought of earlier. :) Still shouldn't be absurdly hard to implement. I'm adding bug 7645 "Validate SVG content" as a dep for this...

And yes, would be very happy if Inkscape would start defaulting to embedding rasters when you add them!

M8R-udfkkf wrote:

Although the <image> element is disabled for thumbnailing, if the user clicks on the thumbnail to enlarge the image, or is given a link to an image, the <image> element is still present.

As SVG's are rendered in firefox, an image with a "xlink:href" to a malicious image file would still go through. Or at least the user's IP would be revealed.

As an example, see http://upload.wikimedia.org/wikipedia/commons/archive/9/9b/20100517130343!Kyokuryu-kai.svg in which you can see the google logo in the background. This is done by adding

<image
   xlink:href="http://www.google.com/intl/en_ALL/images/srpr/logo1w.png"
   x="-55.373806"
   y="-55.316906"
   width="1100"
   height="1100"
   id="image2888" />

All SVGs with "xlink:href" should be marked with some type of warning, or the "xlink:href" stripped or commented out.

There is now an SVGParser in SVGMetadataExtractor.

I guess we could switch the SVG upload filter to this new mediahandler filter and have it look for xlink:href uri's. We could then warn when a file should not be uploaded. It's not hard to do I guess..

giving SVG bugs back to the pool.

Switching to Wikimedia/SVG Rendering component.

Currently the image-tag get stripped, which made the SVG useless (upload). So I recommend to made a big warning (as reported on Commons: VP).

This comment was removed by JoKalliauer.
jijiki claimed this task.
jijiki subscribed.

Closing, please reopen if this is still an issue.

@jijiki : The Problem is not persisting for uploaded files(after ?action=purge, since I guess ~2012), but new files can't be uploaded any more, see first Bug in https://commons.wikimedia.org/wiki/Commons:Commons_SVG_Checker/KnownBugs .

There are two different examples, with different reasons:

<a>-Elemente können nur auf Daten verlinken (href): (eingebettete Datei), http://, https:// oder fragment-Ziele (#, gleiches Dokument). Für andere Elemente wie <image> sind nur data: und fragment erlaubt. Versuche beim Exportieren deiner SVG-Datei Bilder einzubinden. <image http://www.w3.org/1999/xlink:href="blankmap-world-2009.png"> gefunden.

href zu unsicheren Daten gefunden: URI-Ziel <image http://www.w3.org/1999/xlink:href="data:;base64,/9j/..."> in der hochgeladenen SVG-Datei.


https://commons.wikimedia.org/wiki/File:Long_Haul_Flights_Tambo.svg contains a external image, and therefore the image can't be found (except if would have the same name on Commons, but thats anyway not implemented)

in https://commons.wikimedia.org/wiki/File:Europe.svg the mimetype is missing xlink:href="data:;base64, should be replaced by xlink:href="data:image/jpeg;base64, (I often also see xlink:href="data:image/jpg;base64,(jpg instead of jpeg) which is also incorrect and won't be rendered (in newer librsvg-versions since approx 2015) and can't be uploaded any more

That sounds like its working as it should (?)

That sounds like its working as it should (?)

yes sorry, I did not write everything I had intented to write. (Also because I found T193929 , that changed my intention.)

The second bug in T5537#5794820 occours several times (e.g. SVG_help) and has one bug-descrription on it's own ( T193929 ).

The first one in T5537#5794820 does not have a own bug-description, but the error-mesage might be that clear that we might don't need a bug-description-page here. (I know it is not a bug, but at help-pages you should refer to a central proper-bug description, that the users understand the problem.)

Since I found T193929 I woul keep everything as it is. (sorry for briging up this old issue again.)