SECURITY: force IM decoder based on file extension

app/models/optimized_image.rb

@@ -118,6 +118,14 @@ class OptimizedImage < ActiveRecord::Base
     end
   end
 
+  IM_DECODERS ||= /\A(jpe?g|png|tiff?|bmp|ico)\z/i
+
+  def self.prepend_decoder!(path)
+    extension = File.extname(path)[1..-1]
+    raise Discourse::InvalidAccess unless extension[IM_DECODERS]
+    "#{extension}:#{path}"
+  end
+
   def self.thumbnail_or_resize
     SiteSetting.strip_image_metadata ? "thumbnail" : "resize"
   end
@@ -125,6 +133,9 @@ class OptimizedImage < ActiveRecord::Base
   def self.resize_instructions(from, to, dimensions, opts = {})
     ensure_safe_paths!(from, to)
 
+    prepend_decoder!(from)
+    prepend_decoder!(to)
+
     # NOTE: ORDER is important!
     %W{
       convert
@@ -159,6 +170,9 @@ class OptimizedImage < ActiveRecord::Base
   def self.crop_instructions(from, to, dimensions, opts = {})
     ensure_safe_paths!(from, to)
 
+    prepend_decoder!(from)
+    prepend_decoder!(to)
+
     %W{
       convert
       #{from}[0]
@@ -191,6 +205,9 @@ class OptimizedImage < ActiveRecord::Base
   def self.downsize_instructions(from, to, dimensions, opts = {})
     ensure_safe_paths!(from, to)
 
+    prepend_decoder!(from)
+    prepend_decoder!(to)
+
     %W{
       convert
       #{from}[0]

lib/upload_creator.rb

@@ -135,13 +135,19 @@ class UploadCreator
   def convert_to_jpeg!
     jpeg_tempfile = Tempfile.new(["image", ".jpg"])
 
-    OptimizedImage.ensure_safe_paths!(@file.path, jpeg_tempfile.path)
+    from = @file.path
+    to = jpeg_tempfile.path
+
+    OptimizedImage.ensure_safe_paths!(from, to)
+
+    OptimizedImage.prepend_decoder!(from)
+    OptimizedImage.prepend_decoder!(to)
 
     begin
-      execute_convert(@file, jpeg_tempfile)
+      execute_convert(from, to)
     rescue
       # retry with debugging enabled
-      execute_convert(@file, jpeg_tempfile, true)
+      execute_convert(from, to, true)
     end
 
     # keep the JPEG if it's at least 15% smaller
@@ -155,15 +161,18 @@ class UploadCreator
     end
   end
 
-  def execute_convert(input_file, output_file, debug = false)
+  def execute_convert(from, to, debug = false)
-    command = ['convert', input_file.path,
+    command = [
-               '-auto-orient',
+      "convert",
-               '-background', 'white',
+      from,
-               '-interlace', 'none',
+      "-auto-orient",
-               '-flatten',
+      "-background white",
-               '-quality', SiteSetting.png_to_jpg_quality.to_s]
+      "-interlace none",
-    command << '-debug' << 'all' if debug
+      "-flatten",
-    command << output_file.path
+      "-quality #{SiteSetting.png_to_jpg_quality}"
+    ]
+    command << "-debug all" if debug
+    command << to
 
     Discourse::Utils.execute_command(*command, failure_message: I18n.t("upload.png_to_jpg_conversion_failure_message"))
   end
@@ -208,8 +217,13 @@ class UploadCreator
   end
 
   def fix_orientation!
-    OptimizedImage.ensure_safe_paths!(@file.path)
+    path = @file.path
-    Discourse::Utils.execute_command('convert', @file.path, '-auto-orient', @file.path)
+
+    OptimizedImage.ensure_safe_paths!(path)
+    OptimizedImage.prepend_decoder!(path)
+
+    Discourse::Utils.execute_command('convert', path, '-auto-orient', path)
+
     extract_image_info!
   end
 

spec/models/optimized_image_spec.rb

@@ -93,7 +93,7 @@ describe OptimizedImage do
       }.not_to raise_error
     end
 
-    it "raises nothing on paths" do
+    it "raises InvalidAccess error on paths" do
       expect {
         OptimizedImage.ensure_safe_paths!("/a.png", "/b.png", "c.png")
       }.to raise_error(Discourse::InvalidAccess)