dragonfly
dragonfly copied to clipboard
markevans
Lack of magic byte file type detection / ImageTragick mitigation
I'm currently evaluating different file upload solutions for Rails and as far as I can tell Dragonfly solely relies on the file extension to identify the content type of files. Given the recent ImageTragick vulnerability (CVE-2016–3714) we're reminded that blindly trusting user input/files and passing it along for processing is a bad thing. Benign use cases like #386 and #434 would also benefit from from getting the correct mime type by inspecting magic bytes before doing anything with it.
The code has an example for #shell_eval of using the file command, but it is only used for the also vulnerable identify command in the ImageMagick plugin.
Some references on how other libraries dealt with similar issues:
- Egor Homakov's post about getting XSS/RCE by downloading a crafted JPG file with Paperclip and the follow-up Thoughtbot blog post
- https://github.com/thoughtbot/paperclip/blob/master/lib/paperclip/content_type_detector.rb
- https://github.com/carrierwaveuploader/carrierwave/pull/1934
hi there - thanks for the heads up - yes, this is something that needs looking into. it actually used to use the file command automatically but that incurred other problems (maybe because of the way it was implemented). I'll try to come back to this in the next week or so (I'm incredibly busy over the next few days)
Hi Mark, thanks for your reply and your work on this gem. I've dug in the history and it was removed in 4b0a43e2dd8b5733529aff53b8f0c948cb867e3a although the commit message doesn't mention any problems it would have caused.
Those other two gems use the #by_magic method from https://github.com/minad/mimemagic which is probably less prone to edge cases than calling shell commands. Paperclip uses file as a fallback.
In our project we patched this vulnerability by tweaking ImageMagick directly... do you think that doesn't suffice, or it's just a paranoid check to avoid generating more vectors of attack?
