Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageMagick exploit: cve-2016-3714 "magic byte" validation #1933

Closed
timcheadle opened this issue May 4, 2016 · 23 comments
Closed

ImageMagick exploit: cve-2016-3714 "magic byte" validation #1933

timcheadle opened this issue May 4, 2016 · 23 comments

Comments

@timcheadle
Copy link

timcheadle commented May 4, 2016

This article alerts that a new ImageMagick exploit is being used in the wild and describes the following fixes:

  • Using a policy file to limit possible attack vectors
  • Checking the "magic bytes" at the beginning image files to validate that they are actually image files

Can someone provide an update as to whether or not this affects CarrierWave and, if so, what steps are being taken or have been taken to mitigate the vulnerability?

@timcheadle timcheadle changed the title cve-2016-3714 "magic byte" validation ImageMagick exploit: cve-2016-3714 "magic byte" validation May 4, 2016
@d4rky-pl
Copy link

d4rky-pl commented May 4, 2016

Carrierwave uses mime-types gem which detect content type based on filename. I've already flagged this as potential security issue over a year ago (#1543) but the consensus at the time was that it's outside the scope of this project and a custom processor would be preferable.

@thiagofm
Copy link
Member

thiagofm commented May 4, 2016

@d4rky-pl given the current issue, do you mind giving it a try with your fix? I was also looking at a solution yesterday, but didn't know about the file from unix solution.

I would like to leave this outside carrierwave, but given the current issue...

We'll do what we can to protect our users from exploits. If it has to be some sort of temporary patch, I can take care of maintaining it and making sure it transition to another gem, no problem.

Thanks

@fqueiruga
Copy link

What approaches are being considered to detect the content type? I'd use something like mimemagick rather than calling the file command. The reason for this is that in some production environments these kind of syscalls are disabled.

What are your thoughts on this?

@namxam
Copy link

namxam commented May 4, 2016

From what I read, this is also the approach paperclip is using: https://github.com/thoughtbot/paperclip/blob/v4.3.6/lib/paperclip/content_type_detector.rb#L69-L72

@thiagofm
Copy link
Member

thiagofm commented May 4, 2016

The mimemagick approach seems good, anybody is free to submit a PR with a solution. 👍

@alexmuller
Copy link

We've confirmed that a Rails application using carrierwave 0.9.0 is vulnerable to CVE-2016-3714 with ImageMagick 8:6.7.7.10-6ubuntu3.

@locriani
Copy link
Contributor

locriani commented May 4, 2016

I created a PR to fix this issue: #1934

@fqueiruga
Copy link

Will there be a backport for older versions of this fix?

@marksweston
Copy link

Anyone looking for immediate solutions might well take a look at https://github.com/DarthSim/carrierwave-bombshelter

@thiagofm
Copy link
Member

thiagofm commented May 5, 2016

@fqueiruga yes there will be a backport for 0.11, for older versions, feel free to backport it and I'll merge it and generate a new release.

As of now I recommend people to use https://github.com/DarthSim/carrierwave-bombshelter

We're in the process of adding mitigation for this issue in a new release of 0.11

@thiagofm
Copy link
Member

thiagofm commented May 5, 2016

Hello everyone.

The 0.11.2 carrierwave gem version has been published -- Please upgrade and provide feedback if it does work for you.

Don't forget to check the steps provided in: https://github.com/carrierwaveuploader/carrierwave/tree/0.11-stable#cve-2016-3714-imagetragick in order to shield your application from the "CVE-2016-3714 ImageTragick" vunerability.

A BIG THANKS to @locriani who took his time to submit a patch.

For those following the master branch(not really recommended), stick to https://github.com/DarthSim/carrierwave-bombshelter and perhaps also join this discussion: #1934

@myers
Copy link

myers commented May 5, 2016

Please upgrade and provide feedback if it does work for you.

It doesn't work for me. I don't think the content_type_whitelist method of the uploaders that is talked about in the README is ever being called seeing as I had it raise an exception and then attempted an upload.

This commit doesn't seem to add any call to content_type_whitelist

@thiagofm
Copy link
Member

thiagofm commented May 5, 2016

@locriani can you give a read here? Cheers.

@locriani
Copy link
Contributor

locriani commented May 5, 2016

I'll take a look shortly.

@locriani
Copy link
Contributor

locriani commented May 5, 2016

I made the mistake of assuming I could backport cleanly - the content_type_whitelist / blacklist do not exist in the 0.11 branch. I'll create an updated PR that brings that functionality to 0.11

@thiagofm
Copy link
Member

thiagofm commented May 6, 2016

Carrierwave 0.11.2 released, please provide feedback!

@cjbutcher
Copy link

Haven't tested against the vulnerability in question but no problems with 0.11.2 as of yet 👍🏽

@myers
Copy link

myers commented May 10, 2016

I have tested this. You get this exception:

#<CarrierWave::IntegrityError: translation missing: en.errors.messages.content_type_whitelist_error>

I suspect the translation files are missing a key.

@pic
Copy link

pic commented May 13, 2016

detecting the content-type only using the magic number with MimeMagic seems too general to me.
For example now .xslx files have application/zip content type and I guess that it is the same for all zipped XML formats.

@aurcioli-handy
Copy link

If we use the policy.xml fix but using an older version (0.9 or 0.10) are we still vulnerable? The information about the vulnerability of Carrierwave to imagetragick is not well communicated.

@mvz
Copy link

mvz commented Oct 30, 2016

The solution with content_type_whitelist is not working for me because I want to allow any type of file to be uploaded, but only process it if it actually is an image.

I'm now solving that with the following helper method, but it is sub-optimal since it uses a private method:

  def image?(new_file)
    mime_magic_content_type = new_file.send :mime_magic_content_type
    mime_magic_content_type && mime_magic_content_type.include?('image')
  end

@mvz
Copy link

mvz commented Dec 29, 2016

For carrierwave 1.0.0, it would be nice if MagicMimeBlacklist#extract_content_type was available directly.

@mshibuya
Copy link
Member

#1934 is in the master now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests