Page MenuHomePhabricator

Support SVG interactivity and animation in media-viewer
Open, LowPublic

Description

When a dynamic SVG (using CSS and/or SMIL) is opened in Media Viewer a thumbnail is shown rather than the SVG itself. This is not the behaviour expected by a naive user.

Event Timeline

Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 25 2016, 3:03 PM
MartinK added a subscriber: MartinK.
Cmglee renamed this task from Support SVG animations in media-viewer to Support SVG iteractivity and animation in media-viewer.Jun 26 2016, 9:40 AM
Cmglee updated the task description. (Show Details)
Aklapper renamed this task from Support SVG iteractivity and animation in media-viewer to Support SVG interactivity and animation in media-viewer.Jun 28 2016, 7:28 PM

As has been discussed on similar tickets in the past, I can't see any feasible way of doing this without violating our security policies for Wikimedia sites, which removes a great deal of the desire for this change. Am I missing something?

Tgr added a subscriber: Tgr.Jun 29 2016, 11:07 AM

We already sanitize CSS in HTML (and soon CSS in script tags in TemplateStyles), I would not call extending that to SVG infeasible (although it might be more work than the value we get out of it, given that relatively few users have the skills to make use of them).

We already sanitize CSS in HTML (and soon CSS in script tags in TemplateStyles), I would not call extending that to SVG infeasible (although it might be more work than the value we get out of it, given that relatively few users have the skills to make use of them).

ISTR ~85% of all serious WMF security bugs last year were related to SVG files. I'm not so convinced it is feasible. Other projects look at us and run away screaming…

Tgr added a subscriber: brion.Jun 29 2016, 11:10 AM

Also, security-wise there is not much difference between allowing a file to be uploaded (which the task description suggests is already the case) and showing it in MediaViewer through an iframe. Supporting the embedding of images in off-domain iframes is again a lot of work but has a wide area of applications, and IIRC @brion is already working on something similar.

Jdforrester-WMF added a subscriber: Gilles.EditedJun 29 2016, 11:32 AM

The MediaViewer part of this is I think essentially done already by @Gilles from the MediaViewer part T132064: Make Media Viewer pluggable for the 3d extension, videos, ….

Assuming I'm right on that, if we re-purpose this to be "Show animated and interactive SVGs inside MediaViewer (and other locations)" I'd say that it is blocked by the same thing as T131723 more than anything else (i.e., sanitising loading of textures/overlays without the complications); the rest should theoretically "just" be plumbing.

Cmglee added a subscriber: Cmglee.Jun 30 2016, 12:13 PM

I'm unconvinced that showing the SVG in a div, iframe or frame will reduce security, as

  1. The user already has the ability to view it using the "Original file" link, and
  2. The file has been checked during upload.
  1. The user already has the ability to view it using the "Original file" link, and

You're right, we should probably put the bright orange warning telling them it's insecure like we do for PDFs.

  1. The file has been checked during upload.

There are SVG files on Commons from many years ago. They bypassed the newer security rules by virtue of existing before they did. The vast majority of files probably would pass the new rules, but…

There are SVG files on Commons from many years ago. They bypassed the newer security rules by virtue of existing before they did. The vast majority of files probably would pass the new rules, but…

Is it possible to run files uploaded before the new filters through the same filters and add them to a category for manual checking? Alternatively, show the warning for only files which fail the new rules.

Jdlrobson triaged this task as Low priority.Jun 30 2016, 8:20 PM
Jdlrobson added a subscriber: Jdlrobson.

This could potentially be an epic.

Tgr added a comment.Jul 1 2016, 1:30 PM

You're right, we should probably put the bright orange warning telling them it's insecure like we do for PDFs.

Would you be willing to draft the equivalent of the PDF warning page?

There are SVG files on Commons from many years ago. They bypassed the newer security rules by virtue of existing before they did. The vast majority of files probably would pass the new rules, but…

Should we re-run the current rules for old files? That should be pretty easy to do.

You're right, we should probably put the bright orange warning telling them it's insecure like we do for PDFs.

Would you be willing to draft the equivalent of the PDF warning page?

Done (but could do with improvement): https://www.mediawiki.org/wiki/Help:Security/SVG_files – spun out as T139168: On SVG files' pages, put the bright orange warning telling them it's insecure, like we do for PDFs.

There are SVG files on Commons from many years ago. They bypassed the newer security rules by virtue of existing before they did. The vast majority of files probably would pass the new rules, but…

Should we re-run the current rules for old files? That should be pretty easy to do.

Yes, though it might give a false sense of security, and it's not totally clear what we'd do if we found that (say) 100k files failed the screen…

Bawolff added a subscriber: Bawolff.Jul 1 2016, 7:13 PM

My opinion is, that its not out of the question to show scripted svgs in an iframe provided they are served from a totally separate domain (not a subdomain as is current), and use CSP to prevent loading of external resources and have some sort of filtering in place (Having them inline just in a div is out of the question, since then they are in the same origin).

I believe Brion has been working on something along those lines

To be clear, I'm not saying such a thing would be for sure ok, the solution proposed would have to be looked at carefully, but its not out of the question.

Additionally, I do not think this is a question for MediaViewer. This is a question for MediaWiki in general. If at some future time we support animated svgs, then MediaViewer should to. If we don't in general, then mediaviewer should not either. MediaViewer should follow what MediaWiki does in regards to how animated svgs are treated

It should also be clarified that current filtering rules ban animation (except maybe SMIL. But Browser support for SMIL is going away. Oh I guess there is CSS animation), so statements like it's ok to show animated svgs if they pass validation doesn't mean much, since validation will ban any svgs that are applicable.

+1 (We talked about this in Esino Lario.)

A capsuled frame for any kind of rich-media-content is absolutely essential for the future development of interactive media content in the Wikimedia Projects - not only SVGs but also 360°-Panoramas, Interactive Videos, Turntables, etc.

This is the prerequisite for catching up with other plattforms in terms of media content.

Cmglee added a comment.Jul 2 2016, 8:01 PM

+1 agree with MartinK.

Re SVG in particular, I think there are 2 issues:

  1. Old ones possibly containing Javascript – possible security issue, remedied by refiltering old files and using different domain.
  1. Those containing Javascript (at least passing current filters) – less of an issue, but use different domain just in case.

Could a hidden, non-editable category be added by a filter e.g. "sanitized_1.0.0" so MediaViewer can natively show those acceptable versions? If it's later found that that filter version lets through a vulnerability, the MediaViewer version blacklist can be updated.

Menner added a subscriber: Menner.Dec 4 2016, 4:35 PM
Ramsey-WMF moved this task from Untriaged to Tracking on the Multimedia board.Nov 28 2017, 9:04 PM
Tgr removed a subscriber: Tgr.Jul 9 2019, 6:03 PM