Mitigate against CVE-2016-3714 (ImageTragick)

CHANGELOG.md

@@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 * Add Indonesian, Taiwanese and Chinese i18n translations for errors (@saveav, @st0012, @msyesyan)
 
 ### Changed
+* Use the MimeMagic gem to inspect file headers for the mime type. This allows for mitigation of CVE-2016-3714, in combination with a `content_type_whitelist`. (@locriani [#1934](https://github.com/carrierwaveuploader/carrierwave/pull/1934))
 * [BREAKING CHANGE] Allow non-ASCII filename by default (@shuhei [#1772](https://github.com/carrierwaveuploader/carrierwave/pull/1772))
 * [BREAKING CHANGE] `to_json` behavior changed when serializing an uploader (@jnicklas and @lisarutan [#1481](https://github.com/carrierwaveuploader/carrierwave/pull/1481))
 * [BREAKING CHANGE] Rename `extension_white_list` ~> `extension_whitelist` (@mehlah [#1819](https://github.com/carrierwaveuploader/carrierwave/pull/1819))

README.md

@@ -245,6 +245,22 @@ class NoJsonUploader < CarrierWave::Uploader::Base
 end
 ```
 
+### CVE-2016-3714 (ImageTragick)
+This version of CarrierWave has the ability to mitigate CVE-2016-3714. However, you **MUST** set a content_type_whitelist in your uploaders for this protection to be effective, and you **MUST** either disable ImageMagick's default SVG delegate or use the RSVG delegate for SVG processing.
+
+
+A valid whitelist that will restrict your uploader to images only, and mitigate the CVE is:
+
+```ruby
+class MyUploader < CarrierWave::Uploader::Base
+  def content_type_whitelist
+    [/image\//]
+  end
+end
+```
+
+**WARNING**: A `content_type_whitelist` is the only form of whitelist or blacklist supported by CarrierWave that can effectively mitigate against CVE-2016-3714. Use of `extension_type_whitelist` will not inspect the file headers, and thus still leaves your application open to the vulnerability.
+
 ### Filenames and unicode chars
 
 Another security issue you should care for is the file names (see

carrierwave.gemspec

@@ -24,6 +24,8 @@ Gem::Specification.new do |s|
   s.add_dependency "activesupport", ">= 4.0.0"
   s.add_dependency "activemodel", ">= 4.0.0"
   s.add_dependency "mime-types", ">= 1.16"
+  s.add_dependency "mimemagic", ">= 0.3.0"
+
   if RUBY_ENGINE == 'jruby'
     s.add_development_dependency 'activerecord-jdbcpostgresql-adapter'
   else

lib/carrierwave/sanitized_file.rb

@@ -1,5 +1,6 @@
 require 'pathname'
 require 'active_support/core_ext/string/multibyte'
+require 'mimemagic'
 
 begin
   # Use mime/types/columnar if available, for reduced memory usage
@@ -264,12 +265,10 @@ def to_file
     # [String] the content type of the file
     #
     def content_type
-      return @content_type if @content_type
-      if @file.respond_to?(:content_type) and @file.content_type
-        @content_type = @file.content_type.to_s.chomp
-      elsif path
-        @content_type = ::MIME::Types.type_for(path).first.to_s
-      end
+      @content_type ||=
+        existing_content_type ||
+        mime_magic_content_type ||
+        mime_types_content_type
     end
 
     ##
@@ -329,6 +328,22 @@ def sanitize(name)
       return name.mb_chars.to_s
     end
 
+    def existing_content_type
+      if @file.respond_to?(:content_type) && @file.content_type
+        @file.content_type.to_s.chomp
+      end
+    end
+
+    def mime_magic_content_type
+      MimeMagic.by_magic(File.open(path)).try(:type) if path
+    rescue Errno::ENOENT
+      nil
+    end
+
+    def mime_types_content_type
+      ::MIME::Types.type_for(path).first.to_s if path
+    end
+
     def split_extension(filename)
       # regular expressions to try for identifying extensions
       extension_matchers = [

spec/sanitized_file_spec.rb

@@ -202,6 +202,21 @@
 
       expect(sanitized_file.content_type).to eq("image/jpeg")
     end
+
+    it "does not allow spoofing of the mime type" do
+      file = File.open(file_path("zip.png"))
+
+      sanitized_file = CarrierWave::SanitizedFile.new(file)
+      expect { sanitized_file.content_type }.not_to raise_error
+
+      expect(sanitized_file.content_type).to eq("application/zip")
+    end
+
+    it "does not raise an error if the path is not present" do
+      sanitized_file = CarrierWave::SanitizedFile.new(nil)
+
+      expect { sanitized_file.content_type }.not_to raise_error
+    end
   end
 
   describe "#content_type=" do

spec/uploader/content_type_blacklist_spec.rb

@@ -31,7 +31,7 @@
         end
 
         it "raises an integrity error if the file has a blacklisted content type" do
-          allow(uploader).to receive(:content_type_blacklist).and_return(['image/gif'])
+          allow(uploader).to receive(:content_type_blacklist).and_return(['image/png'])
 
           expect { uploader.cache!(ruby_file) }.to raise_error(CarrierWave::IntegrityError)
         end

spec/uploader/content_type_whitelist_spec.rb

@@ -25,7 +25,7 @@
         let(:bork_file) { File.open(file_path('bork.txt')) }
 
         it "does not raise an integrity error when the file has a whitelisted content type" do
-          allow(uploader).to receive(:content_type_whitelist).and_return(['image/gif'])
+          allow(uploader).to receive(:content_type_whitelist).and_return(['image/png'])
 
           expect { uploader.cache!(ruby_file) }.not_to raise_error
         end