From cc0aab2e59eca0541bb1fd311f324a8d8c1968d0 Mon Sep 17 00:00:00 2001
From: Oleksii Kriuchykhin <okriuchykhin@biosistemika.com>
Date: Mon, 28 Nov 2016 10:08:54 +0100
Subject: [PATCH 1/2] Improve regexp for image whitelisting [SCI-736]

---
 app/controllers/assets_controller.rb | 5 +++--
 app/models/asset.rb                  | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb
index 3b2155550..a81b6b03b 100644
--- a/app/controllers/assets_controller.rb
+++ b/app/controllers/assets_controller.rb
@@ -114,8 +114,9 @@ class AssetsController < ApplicationController
       fields: s3_post.fields
     })
 
-    if (asset.file_content_type =~
-      %r{/^image\/#{Constants::WHITELISTED_IMAGE_TYPES.join("|")}/}) == 0
+    condition = %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES)}}
+
+    if condition === asset.file_content_type
       asset.file.options[:styles].each do |style, option|
         s3_post = S3_BUCKET.presigned_post(
           key: asset.file.path(style)[1..-1],
diff --git a/app/models/asset.rb b/app/models/asset.rb
index 50b91b5e5..ec4bec03d 100644
--- a/app/models/asset.rb
+++ b/app/models/asset.rb
@@ -138,8 +138,8 @@ class Asset < ActiveRecord::Base
   end
 
   def is_image?
-    !(file.content_type =~
-      %r{/^image\/#{Constants::WHITELISTED_IMAGE_TYPES.join("|")}/}).nil?
+    %r{^image/#{Regexp.union(Constants::WHITELISTED_IMAGE_TYPES)}} ===
+      file.content_type
   end
 
   def text?

From d8f8e01a46e0969e287da96931d38eb9b36544da Mon Sep 17 00:00:00 2001
From: Oleksii Kriuchykhin <okriuchykhin@biosistemika.com>
Date: Mon, 28 Nov 2016 11:13:20 +0100
Subject: [PATCH 2/2] Code cleanup, added new image types to whitelist
 [SCI-736]

---
 app/models/asset.rb              | 12 +-----------
 config/initializers/constants.rb |  4 +++-
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/app/models/asset.rb b/app/models/asset.rb
index ec4bec03d..ad2832c56 100644
--- a/app/models/asset.rb
+++ b/app/models/asset.rb
@@ -19,7 +19,7 @@ class Asset < ActiveRecord::Base
   # Should be checked for any security leaks
   do_not_validate_attachment_file_type :file
 
-  before_file_post_process :allow_styles_on_images
+  before_file_post_process :is_image?
 
   # Asset validation
   # This could cause some problems if you create empty asset and want to
@@ -298,16 +298,6 @@ class Asset < ActiveRecord::Base
     cache
   end
 
-  protected
-
-  # Checks if attachments is an image (in post processing imagemagick will
-  # generate styles)
-  def allow_styles_on_images
-    if !(file.content_type =~ %r{^(image|(x-)?application)/(x-png|pjpeg|jpeg|jpg|png|gif)$})
-      return false
-    end
-  end
-
   private
 
   def filter_paperclip_errors
diff --git a/config/initializers/constants.rb b/config/initializers/constants.rb
index 57b75dee8..5c03e21c3 100644
--- a/config/initializers/constants.rb
+++ b/config/initializers/constants.rb
@@ -201,7 +201,9 @@ class Constants
     'text/plain'
   ].freeze
 
-  WHITELISTED_IMAGE_TYPES = ['gif', 'jpeg', 'png', 'svg+xml', 'bmp'].freeze
+  WHITELISTED_IMAGE_TYPES = [
+    'gif', 'jpeg', 'pjpeg', 'png', 'x-png', 'svg+xml', 'bmp'
+  ].freeze
 
   # Very basic regex to check for validity of emails
   BASIC_EMAIL_REGEX = /^[^@]+@[^@]+\.[^@]+$/