Merge pull request #1936 from locriani/0.11-cve-2016-3714

Backport fix from PR #1934 to 0.11

README.md

@@ -161,6 +161,22 @@ class MyUploader < 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

@@ -28,6 +28,7 @@ Gem::Specification.new do |s|
   s.add_dependency "activemodel", ">= 3.2.0"
   s.add_dependency "json", ">= 1.7"
   s.add_dependency "mime-types", ">= 1.16"
+  s.add_dependency "mimemagic", ">= 0.3.0"
 
   s.add_development_dependency "pg"
   s.add_development_dependency "rails", ">= 3.2.0"

lib/carrierwave/sanitized_file.rb

@@ -3,6 +3,7 @@
 require 'pathname'
 require 'active_support/core_ext/string/multibyte'
 require 'mime/types'
+require 'mimemagic'
 
 module CarrierWave
 
@@ -244,12 +245,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
 
     ##
@@ -309,6 +308,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

@@ -197,6 +197,21 @@
       @sanitized_file = CarrierWave::SanitizedFile.new('llama.jpg')
       @sanitized_file.content_type.should == 'image/jpeg'
     end
+
+    it 'does not allow spoofing of the mime type' do
+      file = File.open(file_path('jpg.png'))
+
+      sanitized_file = CarrierWave::SanitizedFile.new(file)
+      lambda { sanitized_file.content_type }.should_not raise_error
+
+      sanitized_file.content_type.should == 'image/jpeg'
+    end
+
+    it 'does not raise an error if the path is not present' do
+      sanitized_file = CarrierWave::SanitizedFile.new(nil)
+
+      lambda { sanitized_file.content_type }.should_not raise_error
+    end
   end
 
   describe "#content_type=" do