Ensure uploaded images are image files

Consolidated catalog uploads and icon uploads.
Checking image magic numbers to ensure it is a png or jpg

Resolves issue where user could upload a zip bomb

app/controllers/catalog_controller.rb

@@ -3,6 +3,7 @@ class CatalogController < ApplicationController
   include Mixins::ServiceDialogCreationMixin
   include Mixins::BreadcrumbsMixin
   include Mixins::AutomationMixin
+  include Mixins::ImageValidationMixin
 
   before_action :check_privileges
   before_action :get_session_data
@@ -540,7 +541,7 @@ def st_upload_image
     elsif params[:upload] && params[:upload][:image] &&
           params[:upload][:image].respond_to?(:read)
       ext = params[:upload][:image].original_filename.split(".").last.downcase
-      if !%w[png jpg].include?(ext)
+      if !valid_image_file?(params[:upload][:image])
         msg = _("Custom Image must be a .png or .jpg file")
         err = true
       else

app/controllers/mixins/image_validation_mixin.rb

@@ -0,0 +1,32 @@
+module Mixins
+  module ImageValidationMixin
+    private
+
+    # @param file request parameter for a file
+    # @param ext if present, the only extension supported (default: nil / accept all extensions)
+    def valid_image_file?(file, type = nil)
+      ext = File.ext_name(file.original_filename).downcase
+      return false if type && ext != type
+
+      valid_magic_number =
+        case ext
+        when "ico"
+          "\x00\x00\x01\x00".force_encoding("ASCII-8BIT")
+        when "png"
+          "\x89PNG\r\n\x1A\n".force_encoding("ASCII-8BIT")
+        when "jpg"
+          "\xff\xd8\xff".force_encoding("ASCII-8BIT")
+        else
+          return false
+        end
+
+      magic_number = File.open(file.tempfile.path, 'rb') do |f|
+        f.readpartial(valid_magic_number.size)
+      end
+
+      magic_number == valid_magic_number
+    rescue EOFError
+      return false
+    end      
+  end
+end

app/controllers/ops_controller/settings/upload.rb

@@ -1,5 +1,6 @@
 module OpsController::Settings::Upload
   extend ActiveSupport::Concern
+  include Mixins::ImageValidationMixin
 
   def upload_logo
     assert_privileges("ops_settings")
@@ -31,7 +32,7 @@ def upload_favicon
 
   def upload_logos(file, field, text, type)
     if field && field[:logo] && field[:logo].respond_to?(:read)
-      unless valid_file?(field[:logo], type)
+      unless valid_image_file?(field[:logo], type)
         add_flash(_("%{image} must be a .%{type} file") % {:image => text, :type => type}, :error)
       else
         File.open(file, "wb") { |f| f.write(field[:logo].read) }
@@ -115,28 +116,4 @@ def logo_dir
     Dir.mkdir(dir) unless dir.exist?
     dir.to_s
   end
-
-  def valid_file?(file, type)
-    ext = file.original_filename.split(".").last.downcase
-    return false unless ext == type
-
-    case type
-    when "ico"
-      valid_magic_number = "\x00\x00\x01\x00".force_encoding("ASCII-8BIT")
-    when "png"
-      valid_magic_number = "\x89PNG\r\n\x1A\n".force_encoding("ASCII-8BIT")
-    else
-      return false
-    end
-
-    magic_number = File.open(file.tempfile.path, 'rb') do |f|
-      begin
-        f.readpartial(valid_magic_number.size)
-      rescue EOFError
-        return false
-      end
-    end
-
-    magic_number == valid_magic_number
-  end
 end