Check for spoofing of files without an extension

While using the Paperclip gem, we noticed during some ad-hoc testing
that if you do not supply an extension when uploading a file, Paperclip
effectively skipped it's spoofing check, which allowed potentially
dangerous files to slip through into your application.

This addresses that by moving the checks around a little bit and only
testing against the extension when there is one.

NEWS

@@ -2,6 +2,7 @@ master:
 
 
 * Improvement: Better handling of the content-disposition header. Now supports file name that is either
 * Improvement: Better handling of the content-disposition header. Now supports file name that is either
   enclosed or not in double quotes and is case insensitive as per RC6266 grammar
   enclosed or not in double quotes and is case insensitive as per RC6266 grammar
+* Improvement: Files without an extension will now be checked for spoofing attempts
 
 
 6.0.0 (2018-03-09):
 6.0.0 (2018-03-09):
 
 

lib/paperclip/media_type_spoof_detector.rb

@@ -11,7 +11,7 @@ module Paperclip
     end
     end
 
 
     def spoofed?
     def spoofed?
-      if has_name? && has_extension? && media_type_mismatch? && mapping_override_mismatch?
+      if has_name? && media_type_mismatch? && mapping_override_mismatch?
         Paperclip.log("Content Type Spoof: Filename #{File.basename(@name)} (#{supplied_content_type} from Headers, #{content_types_from_name.map(&:to_s)} from Extension), content type discovered from file command: #{calculated_content_type}. See documentation to allow this combination.")
         Paperclip.log("Content Type Spoof: Filename #{File.basename(@name)} (#{supplied_content_type} from Headers, #{content_types_from_name.map(&:to_s)} from Extension), content type discovered from file command: #{calculated_content_type}. See documentation to allow this combination.")
         true
         true
       else
       else
@@ -30,15 +30,18 @@ module Paperclip
     end
     end
 
 
     def media_type_mismatch?
     def media_type_mismatch?
-      supplied_type_mismatch? || calculated_type_mismatch?
+      extension_type_mismatch? || calculated_type_mismatch?
     end
     end
 
 
-    def supplied_type_mismatch?
-      supplied_media_type.present? && !media_types_from_name.include?(supplied_media_type)
+    def extension_type_mismatch?
+      supplied_media_type.present? &&
+        has_extension? &&
+        !media_types_from_name.include?(supplied_media_type)
     end
     end
 
 
     def calculated_type_mismatch?
     def calculated_type_mismatch?
-      !media_types_from_name.include?(calculated_media_type)
+      supplied_media_type.present? &&
+        !calculated_content_type.include?(supplied_media_type)
     end
     end
 
 
     def mapping_override_mismatch?
     def mapping_override_mismatch?

spec/paperclip/media_type_spoof_detector_spec.rb

@@ -58,6 +58,32 @@ describe Paperclip::MediaTypeSpoofDetector do
     end
     end
   end
   end
 
 
+  context "GIF file named without extension, but we're told GIF" do
+    let(:file) { File.open(fixture_file("animated")) }
+    let(:spoofed?) do
+      Paperclip::MediaTypeSpoofDetector.
+        using(file, "animated", "image/gif").
+        spoofed?
+    end
+
+    it "accepts the file" do
+      assert !spoofed?
+    end
+  end
+
+  context "GIF file named without extension, but we're told HTML" do
+    let(:file) { File.open(fixture_file("animated")) }
+    let(:spoofed?) do
+      Paperclip::MediaTypeSpoofDetector.
+        using(file, "animated", "text/html").
+        spoofed?
+    end
+
+    it "rejects the file" do
+      assert spoofed?
+    end
+  end
+
   it "does not reject if content_type is empty but otherwise checks out" do
   it "does not reject if content_type is empty but otherwise checks out" do
     file = File.open(fixture_file("empty.html"))
     file = File.open(fixture_file("empty.html"))
     assert ! Paperclip::MediaTypeSpoofDetector.using(file, "empty.html", "").spoofed?
     assert ! Paperclip::MediaTypeSpoofDetector.using(file, "empty.html", "").spoofed?