From 072e3c2ad9b67807389fd612ce14c950282af2c9 Mon Sep 17 00:00:00 2001 From: Zach Gardner Date: Fri, 6 May 2016 17:28:28 -0700 Subject: [PATCH] Do not allow file type spoofing if the header bytes are not recognized --- lib/carrierwave/sanitized_file.rb | 2 +- spec/fixtures/spoof.png | 4 ++++ spec/sanitized_file_spec.rb | 11 +++++++++++ spec/spec_helper.rb | 1 + spec/storage/fog_helper.rb | 2 ++ spec/uploader/proxy_spec.rb | 2 +- 6 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/spoof.png diff --git a/lib/carrierwave/sanitized_file.rb b/lib/carrierwave/sanitized_file.rb index 8ad6b6d5f..f0d231101 100644 --- a/lib/carrierwave/sanitized_file.rb +++ b/lib/carrierwave/sanitized_file.rb @@ -315,7 +315,7 @@ def existing_content_type end def mime_magic_content_type - MimeMagic.by_magic(File.open(path)).try(:type) if path + MimeMagic.by_magic(File.open(path)).try(:type) || 'invalid/invalid' if path rescue Errno::ENOENT nil end diff --git a/spec/fixtures/spoof.png b/spec/fixtures/spoof.png new file mode 100644 index 000000000..3d8c915a9 --- /dev/null +++ b/spec/fixtures/spoof.png @@ -0,0 +1,4 @@ +push graphic-context +viewbox 0 0 640 480 +fill 'url(https://example.com/image.jpg";|ls "-la)' +pop graphic-context diff --git a/spec/sanitized_file_spec.rb b/spec/sanitized_file_spec.rb index a39dc14e6..eb3a920e9 100644 --- a/spec/sanitized_file_spec.rb +++ b/spec/sanitized_file_spec.rb @@ -207,6 +207,17 @@ sanitized_file.content_type.should == 'image/jpeg' end + it 'does not allow spoofing of the mime type if the mime type is not detectable' do + file = File.open(file_path('spoof.png')) + + sanitized_file = CarrierWave::SanitizedFile.new(file) + + lambda { sanitized_file.content_type }.should_not raise_error + + sanitized_file.content_type.should_not == 'image/png' + sanitized_file.content_type.should == 'invalid/invalid' + end + it 'does not raise an error if the path is not present' do sanitized_file = CarrierWave::SanitizedFile.new(nil) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a41b3b92f..27747143d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -90,6 +90,7 @@ def stub_stringio(filename, mime_type=nil, fake_name=nil) def stub_file(filename, mime_type=nil, fake_name=nil) f = File.open(file_path(filename)) + f.stub(:content_type) { mime_type } if mime_type return f end end diff --git a/spec/storage/fog_helper.rb b/spec/storage/fog_helper.rb index abfbd40a2..c5d19a111 100644 --- a/spec/storage/fog_helper.rb +++ b/spec/storage/fog_helper.rb @@ -309,6 +309,7 @@ class FogSpec#{fog_credentials[:provider]}Uploader < CarrierWave::Uploader::Base describe "with a valid path" do before do @file = CarrierWave::SanitizedFile.new(file_path('test.jpg')) + @file.file.stub(:content_type) { 'image/jpeg' } @file.should_not be_empty end @@ -318,6 +319,7 @@ class FogSpec#{fog_credentials[:provider]}Uploader < CarrierWave::Uploader::Base describe "with a valid Pathname" do before do @file = CarrierWave::SanitizedFile.new(Pathname.new(file_path('test.jpg'))) + @file.file.stub(:content_type) { 'image/jpeg' } @file.should_not be_empty end diff --git a/spec/uploader/proxy_spec.rb b/spec/uploader/proxy_spec.rb index 5d7d9ba73..d34176fc2 100644 --- a/spec/uploader/proxy_spec.rb +++ b/spec/uploader/proxy_spec.rb @@ -57,7 +57,7 @@ end it "should get the content type when the file has been cached" do - @uploader.cache!(File.open(file_path('test.jpg'))) + @uploader.cache!(File.open(file_path('landscape.jpg'))) @uploader.content_type.should == 'image/jpeg' end