Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Commit

Permalink
Check for spoofing of files without an extension
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
George Walters II authored and mike-burns committed May 25, 2018
1 parent 0d93e0f commit 7eb664f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ master:

* 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
* Improvement: Files without an extension will now be checked for spoofing attempts

6.0.0 (2018-03-09):

Expand Down
13 changes: 8 additions & 5 deletions lib/paperclip/media_type_spoof_detector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def initialize(file, name, content_type)
end

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.")
true
else
Expand All @@ -30,15 +30,18 @@ def has_extension?
end

def media_type_mismatch?
supplied_type_mismatch? || calculated_type_mismatch?
extension_type_mismatch? || calculated_type_mismatch?
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

def calculated_type_mismatch?
!media_types_from_name.include?(calculated_media_type)
supplied_media_type.present? &&
!calculated_content_type.include?(supplied_media_type)
end

def mapping_override_mismatch?
Expand Down
26 changes: 26 additions & 0 deletions spec/paperclip/media_type_spoof_detector_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,32 @@
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
file = File.open(fixture_file("empty.html"))
assert ! Paperclip::MediaTypeSpoofDetector.using(file, "empty.html", "").spoofed?
Expand Down

0 comments on commit 7eb664f

Please sign in to comment.