Merge pull request #3376 from AlchemyCMS/sanitize-filenames

Sanitize filenames before upload

Author: tvdeyen

app/models/alchemy/storage_adapter/dragonfly.rb

@@ -15,6 +15,10 @@ def self.included(base)
 
             has_many :thumbs, class_name: "Alchemy::PictureThumb", dependent: :destroy
 
+            before_save do
+              self.image_file_name = sanitized_filename(image_file_name)
+            end
+
             # Create important thumbnails upfront
             after_create -> { PictureThumb.generate_thumbs!(self) },
               if: :has_convertible_format?
@@ -30,6 +34,10 @@ def self.included(base)
                 write_attribute(:file_mime_type, file.mime_type)
               }
             end
+
+            before_save do
+              self.file_name = sanitized_filename(file_name)
+            end
           end
         end
       end

lib/alchemy/name_conversions.rb

@@ -22,5 +22,11 @@ def convert_to_urlname(name)
     def convert_to_humanized_name(name, suffix)
       name.gsub(/\.#{::Regexp.quote(suffix)}$/i, "").tr("_", " ").strip
     end
+
+    # Sanitizes a given filename by removing directory traversal attempts and HTML entities.
+    def sanitized_filename(file_name)
+      file_name = File.basename(file_name)
+      CGI.escapeHTML(file_name)
+    end
   end
 end

spec/models/alchemy/attachment_spec.rb

@@ -10,6 +10,13 @@ module Alchemy
       build(:alchemy_attachment, file:, name: nil, file_name: nil)
     end
 
+    if Alchemy.storage_adapter.dragonfly?
+      it_behaves_like "having file name sanitization" do
+        subject { Attachment.new(file:) }
+        let(:file_name_attribute) { :file_name }
+      end
+    end
+
     it_behaves_like "a relatable resource",
       resource_name: :attachment,
       ingredient_type: :file

spec/models/alchemy/picture_spec.rb

@@ -10,6 +10,13 @@ module Alchemy
 
     let(:picture) { build(:alchemy_picture, image_file: image_file) }
 
+    if Alchemy.storage_adapter.dragonfly?
+      it_behaves_like "having file name sanitization" do
+        subject { Picture.new(image_file:) }
+        let(:file_name_attribute) { :image_file_name }
+      end
+    end
+
     it_behaves_like "a relatable resource",
       resource_name: :picture,
       ingredient_type: :picture

spec/rails_helper.rb

@@ -34,6 +34,7 @@
 require "alchemy/test_support/shared_uploader_examples"
 require "alchemy/test_support/current_language_shared_examples"
 
+require_relative "support/file_name_examples"
 require_relative "support/hint_examples"
 require_relative "support/custom_news_elements_finder"
 

spec/support/file_name_examples.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.shared_examples_for "having file name sanitization" do
+  describe "file name sanitization" do
+    let(:invalid_file_name) { 'some/../path"<script>alert(1)</script>.png' }
+    let(:sanitized_file_name) { "script&gt;.png" }
+
+    it "sanitizes the file name before saving" do
+      subject.send("#{file_name_attribute}=", invalid_file_name)
+      subject.save
+      expect(subject.send(file_name_attribute)).to eq(sanitized_file_name)
+    end
+  end
+end