Opened 12 years ago
Last modified 8 years ago
#51 closed defect (FIXED)
Handing of bad media types (html!)
|Reported by:||Elrond||Owned by:||Jakob Kramer|
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.
Change History (12)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
|Status:||New → In Progress|
comment:2 by , 12 years ago
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)
comment:3 by , 12 years ago
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.
comment:4 by , 12 years ago
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?
comment:5 by , 12 years ago
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) in ALLOWED: Jakob: I have reassigned back to you for clearance.
comment:6 by , 12 years ago
I've removed the PIL check and made the change Elrond suggested for now, and things seem to submit okay.
comment:7 by , 12 years ago
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.
comment:8 by , 12 years ago
|Milestone:||0.0.3 → 0.0.4|
Targeting for 0.0.4
comment:9 by , 12 years ago
|Milestone:||0.0.4 → 0.0.5|
We release 0.0.4, so I'm bumping this to 0.0.5.
comment:10 by , 12 years ago
|Status:||In Progress → Closed|
Good news, with the recent conversion overhaul this is finished :)
comment:11 by , 11 years ago
The original url for this bug was http://bugs.foocorp.net/issues/324 .
Note: See TracTickets for help on using tickets.