Handing of bad media types (html!)

[Elrond]

Currently you can try to upload a html file.

What happens currebtly? It sticks in the queue, because celery goes
crazy on it.
The good news: It does not end up in a public place.

What should NEVER, ever happen: The file being put in a public
place. It's the best XSS attack to come up with.

Rating this high, because this needs to be right for security
reasons.

[Christopher Allan Webber]

Yeah, I'm not sure how to best check / fix this in all scenarios.
Right now we're just supporting images, so we only need to check
that the thing is an image. We're doing that... PIL has to open the
file, maybe actually checking that the file **can** open is the
best way to make sure that it's a file of the appropriate type?

I'm not sure in the glorious future of multiple media types if we
should add something to the processing interface that does a
security check or not, but it's something to think about.

A related thing is that we need to have a way to track "results" of
the processing and have a panel for users that can help them figure
out why it failed. Right now celery **does** store this
information, but I think this is in the scope of a different but
related ticket.

[Jakob Kramer]

Fix:
[https://gitorious.org/\ :sub:`gandaro/mediagoblin/gandaros-mediagoblin/commits/324-bad-media-types](https://gitorious.org/`\ gandaro/mediagoblin/gandaros-mediagoblin/commits/324-bad-media-types)

[Elrond]

Hi,


1. Added `#361 </issues/361>`_ as related, as browser "upsniffing"
   (or whatever it's called) might be related to this bug.
2. Assigned to cwebber for review/merging. Jakob seems to be happy
   with his branch, I guess
3. Chris: Please review, merge, close, and/or assign back to
   Jakob.

[Christopher Allan Webber]

Hm, okay, I've merged that for now. The way to handle the mimetype
stuff is right on, but loading up and actually checking the image I
think should happen (and will happen) during the processing stage.

I think the ideal system is this two step process:


-  Check the mime type for the media type that's being uploaded
-  If conversion fails somehow or a problem is detected during the
   processing step, an appropriate error is filed that the application
   can know about and that can be displayed in some sort of "upload
   progress" panel.

In the glorious future of multiple media types, media types will
whitelist the accepted mimetypes and will handle the failure stuff
in their own processing functions, but in a standardized way.

I guess we need to handle conversion failures properly though, and
we haven't thought about how to record that properly. Maybe we
should start thinking about that now?

[Elrond]

Hi,
okay, first bug turns up in the security code (on irc, by
"iAlecs"):

guess\_type from mimetypes returns a tuple, the code only wants the
first element. So the code should probably read:

::

    if not guess_type(posted_file.filename)[0] in ALLOWED:

Jakob: I have reassigned back to you for clearance.

[Christopher Allan Webber]

I've removed the PIL check and made the change Elrond suggested for
now, and things seem to submit okay.

[Christopher Allan Webber]

So as for the actual processing detecting if there was a problem
during conversion, we should probably update the state of the task
if a failure happens and have a standard error code like
CONVERSION\_FAILED. So I think that will be round 2 of catching bad
formats, that they're caught in the conversion failure cycle.

[Christopher Allan Webber]

Targeting for 0.0.4

[Will Kahn-Greene]

We release 0.0.4, so I'm bumping this to 0.0.5.

[Christopher Allan Webber]

Good news, with the recent conversion overhaul this is finished :)

[Will Kahn-Greene]

The original url for this bug was ​http://bugs.foocorp.net/issues/324 .
Relations:
#76: related