SECURITY: check magic bytes before using ImageMagick tools
This commit is contained in:
parent
b061ba5c52
commit
df14926e42
2
Gemfile
2
Gemfile
|
@ -61,7 +61,7 @@ gem 'fast_xs'
|
||||||
gem 'fast_xor'
|
gem 'fast_xor'
|
||||||
|
|
||||||
# while we sort out https://github.com/sdsykes/fastimage/pull/46
|
# while we sort out https://github.com/sdsykes/fastimage/pull/46
|
||||||
gem 'fastimage_discourse', require: 'fastimage'
|
gem 'discourse_fastimage', require: 'fastimage'
|
||||||
gem 'aws-sdk', require: false
|
gem 'aws-sdk', require: false
|
||||||
gem 'excon', require: false
|
gem 'excon', require: false
|
||||||
gem 'unf', require: false
|
gem 'unf', require: false
|
||||||
|
|
|
@ -73,6 +73,7 @@ GEM
|
||||||
diff-lcs (1.2.5)
|
diff-lcs (1.2.5)
|
||||||
discourse-qunit-rails (0.0.9)
|
discourse-qunit-rails (0.0.9)
|
||||||
railties
|
railties
|
||||||
|
discourse_fastimage (2.0.0)
|
||||||
docile (1.1.5)
|
docile (1.1.5)
|
||||||
domain_name (0.5.25)
|
domain_name (0.5.25)
|
||||||
unf (>= 0.0.5, < 1.0.0)
|
unf (>= 0.0.5, < 1.0.0)
|
||||||
|
@ -107,7 +108,6 @@ GEM
|
||||||
rake
|
rake
|
||||||
rake-compiler
|
rake-compiler
|
||||||
fast_xs (0.8.0)
|
fast_xs (0.8.0)
|
||||||
fastimage_discourse (1.6.6)
|
|
||||||
ffi (1.9.10)
|
ffi (1.9.10)
|
||||||
flamegraph (0.1.0)
|
flamegraph (0.1.0)
|
||||||
fast_stack
|
fast_stack
|
||||||
|
@ -413,6 +413,7 @@ DEPENDENCIES
|
||||||
byebug
|
byebug
|
||||||
certified
|
certified
|
||||||
discourse-qunit-rails
|
discourse-qunit-rails
|
||||||
|
discourse_fastimage
|
||||||
email_reply_trimmer (= 0.1.3)
|
email_reply_trimmer (= 0.1.3)
|
||||||
ember-rails (= 0.18.5)
|
ember-rails (= 0.18.5)
|
||||||
ember-source (= 1.12.2)
|
ember-source (= 1.12.2)
|
||||||
|
@ -422,7 +423,6 @@ DEPENDENCIES
|
||||||
fast_blank
|
fast_blank
|
||||||
fast_xor
|
fast_xor
|
||||||
fast_xs
|
fast_xs
|
||||||
fastimage_discourse
|
|
||||||
flamegraph
|
flamegraph
|
||||||
foreman
|
foreman
|
||||||
gctools
|
gctools
|
||||||
|
|
|
@ -70,7 +70,7 @@ class Upload < ActiveRecord::Base
|
||||||
def self.create_for(user_id, file, filename, filesize, options = {})
|
def self.create_for(user_id, file, filename, filesize, options = {})
|
||||||
DistributedMutex.synchronize("upload_#{user_id}_#{filename}") do
|
DistributedMutex.synchronize("upload_#{user_id}_#{filename}") do
|
||||||
# do some work on images
|
# do some work on images
|
||||||
if FileHelper.is_image?(filename) && system("identify '#{file.path}' >/dev/null 2>&1")
|
if FileHelper.is_image?(filename) && is_actual_image?(file)
|
||||||
if filename =~ /\.svg$/i
|
if filename =~ /\.svg$/i
|
||||||
svg = Nokogiri::XML(file).at_css("svg")
|
svg = Nokogiri::XML(file).at_css("svg")
|
||||||
w = svg["width"].to_i
|
w = svg["width"].to_i
|
||||||
|
@ -170,6 +170,14 @@ class Upload < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def self.is_actual_image?(file)
|
||||||
|
# due to ImageMagick CVE-2016–3714, use FastImage to check the magic bytes
|
||||||
|
# cf. https://meta.discourse.org/t/imagemagick-cve-2016-3714/43624
|
||||||
|
FastImage.size(file, raise_on_failure: true)
|
||||||
|
rescue
|
||||||
|
false
|
||||||
|
end
|
||||||
|
|
||||||
LARGE_PNG_SIZE ||= 3.megabytes
|
LARGE_PNG_SIZE ||= 3.megabytes
|
||||||
|
|
||||||
def self.should_optimize?(path)
|
def self.should_optimize?(path)
|
||||||
|
|
|
@ -63,7 +63,7 @@ describe Upload do
|
||||||
|
|
||||||
it "computes width & height for images" do
|
it "computes width & height for images" do
|
||||||
ImageSizer.expects(:resize)
|
ImageSizer.expects(:resize)
|
||||||
image.expects(:rewind).twice
|
image.expects(:rewind).times(3)
|
||||||
Upload.create_for(user_id, image, image_filename, image_filesize)
|
Upload.create_for(user_id, image, image_filename, image_filesize)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue