Page MenuHomePhabricator

Provide a mechanism to sanitise user-generated non-STL 3D files to meet security requirements
Open, LowestPublic

Description

All the lovely work on client-side and server-side rendering won't go anywhere if we can't allow files to be uploaded for security reasons.

(Alternatively, could we host the files on something like wmfusercontent.org as we use for Phabricator in an <iframe> that doesn't have access to things we're worried about?)

Event Timeline

Restricted Application added subscribers: Steinsplitter, Matanya, Aklapper. · View Herald TranscriptApr 3 2016, 2:08 PM
matmarex added a subscriber: matmarex.EditedApr 3 2016, 4:30 PM

It doesn't really make sense to think about this unless we know what specific file types we're planning to accept. If I remember, proposals so far involved STL or AMF.

STL is apparently a very simple format. For these files we'd probably only want to verify that it can't be interpreted as some other, more nefarious format (e.g. a valid binary STL that is also a valid executable, or archive) – we already check file extensions against the detected MIME type, so this should require no special work.

AMF is apparently XML-based. For these files we'd probably want the same checks that we do for SVG files – the syntax is valid, no embedded scripts. We'd also need to do a security review of any server-side renderer to ensure it's not vulnerable to the usual attacks via weird XML entities. Edited to add: Ah, it's actually a ZIP archive containing XML. I'm not sure if that makes it much more troublesome, but it's definitely not helping.

brion added a subscriber: brion.Apr 4 2016, 8:41 AM
Tgr added a subscriber: Tgr.Apr 6 2016, 3:27 PM
Restricted Application added a subscriber: Poyekhali. · View Herald TranscriptMay 4 2016, 7:47 PM
Gilles added a subscriber: Gilles.May 8 2016, 1:16 PM

AMF is lower priority. It can be zipped, but doesn't have to be. It's very new, I'm not sure that there are that many pieces of software that support it.

So far the server-side code and client-side are the same. They are extremely simple, merely looking for expected values and ignoring anything unexpected.

At no point are the files interpreted by the client natively in the browser.

If certainly wouldn't hurt to have AMF go through the same XML vulnerability detection as the other XML-based formats (if that's not already the case? there is some common code for XML-based formats).

As for STL, I've never heard of any security issue. It's a very simple format only containing information about geometry. Security issues with file formats tend to appear when there are fancy features, and both AMF and STL are as dumb as it gets.

There will probably be more concern to be had with the many 3d file formats that support external file references (typically, textures). I've looked at AMF and STL first precisely because they don't have those features.

MarkTraceur moved this task from Untriaged to Triaged on the Multimedia board.Dec 5 2016, 5:42 PM

Given that we've decided to only support STL, which is a binary format with no mechanisms for interacting with the outside world, is this task obviated? Do we want to re-title it to deal only with AMF (which we aren't using anyway)?

Indeed, at the moment STL is very safe and doesn't need sanitation. When we venture into supporting additional format, this should be a de facto requirement to launching said support. Up to you to close this task or keep it around for future formats, it's your project now :)

OK, I'll leave this up to @Jdforrester-WMF to decide when he returns to keyboard.

I think we could re-purpose this to "Provide a mechanism to sanitise user-generated non-STL 3D files to meet security requirements" as a blocker to the inevitable request as soon as STL support is released. :-)

Jdforrester-WMF renamed this task from Provide a mechanism to sanitise user-generated 3D files (which filetypes?) to meet security requirements to Provide a mechanism to sanitise user-generated non-STL 3D files to meet security requirements.Feb 23 2017, 7:16 PM
Jdforrester-WMF lowered the priority of this task from Normal to Lowest.
Jdforrester-WMF added a project: Epic.
Ainali added a subscriber: Ainali.Oct 5 2017, 7:01 AM
Quiddity removed a subscriber: Quiddity.Feb 23 2018, 8:04 PM
Quiddity added a subscriber: Quiddity.