CVE-2022-1811: Unrestricted Upload of File with Dangerous Type (CWE-434)
value: Unrestricted Upload of File with Dangerous Type in GitHub repository publify/publify prior to 9.2.9.
CWE description: The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
CVSS v3 base score: 5.4
Published: 2022-05-23T16:16Z
Repository: https://github.com/publify/publify
Language: Ruby
Files changed: publify_core/app/uploaders/resource_uploader.rb, publify_core/spec/controllers/admin/resources_controller_spec.rb
Commit date: 2022-05-22 15:25:53 +0200
Commit message: Refuse html files as resources even if declared to be plain text

Diff: diff --git a/publify_core/app/uploaders/resource_uploader.rb b/publify_core/app/uploaders/resource_uploader.rb
index 2f097417a0..36d5d5a5e4 100644
--- a/publify_core/app/uploaders/resource_uploader.rb
+++ b/publify_core/app/uploaders/resource_uploader.rb
@@ -2,11 +2,11 @@
 
 require "marcel"
 
 class ResourceUploader < CarrierWave::Uploader::Base
   include CarrierWave::MiniMagick
-  before :cache, :check_image_content_type!
+  before :cache, :check_content_type!
 
   def content_type_allowlist
     [%r{image/}, %r{audio/}, %r{video/}, "text/plain"]
   end
 
@@ -35,28 +35,26 @@ class ResourceUploader < CarrierWave::Uploader::Base
   def image?(new_file)
     content_type = new_file.content_type
     content_type&.include?("image")
   end
 
-  def check_image_content_type!(new_file)
-    if image?(new_file)
-      magic_type = mime_magic_content_type(new_file)
-      if magic_type != new_file.content_type
-        raise CarrierWave::IntegrityError, "has MIME type mismatch"
-      end
+  def check_content_type!(new_file)
+    detected_type = if image? new_file
+                      file_content_content_type(new_file)
+                    else
+                      file_content_type(new_file)
+                    end
+    if detected_type != new_file.content_type
+      raise CarrierWave::IntegrityError, "has MIME type mismatch"
     end
   end
 
   private
 
-  # NOTE: This method was adapted from MagicMimeBlacklist#extract_content_type
-  # from CarrierWave 1.0.0 and SanitizedFile#mime_magic_content_type from CarrierWave 0.11.2
-  def mime_magic_content_type(new_file)
-    content_type = nil
-
-    File.open(new_file.path) do |fd|
-      content_type = Marcel::MimeType.for(fd)
-    end
+  def file_content_content_type(new_file)
+    Marcel::MimeType.for Pathname.new(new_file.path)
+  end
 
-    content_type
+  def file_content_type(new_file)
+    Marcel::MimeType.for Pathname.new(new_file.path), name: new_file.filename
   end
 end
diff --git a/publify_core/spec/controllers/admin/resources_controller_spec.rb b/publify_core/spec/controllers/admin/resources_controller_spec.rb
index 85820f686b..2b7fc066e5 100644
--- a/publify_core/spec/controllers/admin/resources_controller_spec.rb
+++ b/publify_core/spec/controllers/admin/resources_controller_spec.rb
@@ -185,10 +185,34 @@ describe Admin::ResourcesController, type: :controller do
           expect(flash[:warning]).not_to be_nil
         end
       end
     end
 
+    context "when attempting to upload an html file as text/plain" do
+      let(:upload) { file_upload("just_some.html", "text/plain") }
+
+      it "does not create a new Resource" do
+        expect { post :upload, params: { upload: upload } }.
+          not_to change(Resource, :count)
+      end
+
+      it "warns the user they can't upload this type of file" do
+        post :upload, params: { upload: upload }
+        result = assigns(:up)
+        expect(result.errors[:upload]).
+          to match_array ["has MIME type mismatch", "can't be blank"]
+      end
+
+      it "sets the flash to failure" do
+        post :upload, params: { upload: upload }
+        aggregate_failures do
+          expect(flash[:success]).to be_nil
+          expect(flash[:warning]).not_to be_nil
+        end
+      end
+    end
+
     context "when uploading nothing" do
       it "does not create a new Resource" do
         expect { post :upload }.
           not_to change(Resource, :count)
       end

Vulnerable code:   before :cache, :check_image_content_type!
  def check_image_content_type!(new_file)
    if image?(new_file)
      magic_type = mime_magic_content_type(new_file)
      if magic_type != new_file.content_type
        raise CarrierWave::IntegrityError, "has MIME type mismatch"
      end
  # NOTE: This method was adapted from MagicMimeBlacklist#extract_content_type
  # from CarrierWave 1.0.0 and SanitizedFile#mime_magic_content_type from CarrierWave 0.11.2
  def mime_magic_content_type(new_file)
    content_type = nil

    File.open(new_file.path) do |fd|
      content_type = Marcel::MimeType.for(fd)
    end
    content_type
Fixed code:   before :cache, :check_content_type!
  def check_content_type!(new_file)
    detected_type = if image? new_file
                      file_content_content_type(new_file)
                    else
                      file_content_type(new_file)
                    end
    if detected_type != new_file.content_type
      raise CarrierWave::IntegrityError, "has MIME type mismatch"
  def file_content_content_type(new_file)
    Marcel::MimeType.for Pathname.new(new_file.path)
  end
  def file_content_type(new_file)
    Marcel::MimeType.for Pathname.new(new_file.path), name: new_file.filename
    context "when attempting to upload an html file as text/plain" do
      let(:upload) { file_upload("just_some.html", "text/plain") }

      it "does not create a new Resource" do
        expect { post :upload, params: { upload: upload } }.
          not_to change(Resource, :count)
      end

      it "warns the user they can't upload this type of file" do
        post :upload, params: { upload: upload }
        result = assigns(:up)
        expect(result.errors[:upload]).
          to match_array ["has MIME type mismatch", "can't be blank"]
      end

      it "sets the flash to failure" do
        post :upload, params: { upload: upload }
        aggregate_failures do
          expect(flash[:success]).to be_nil
          expect(flash[:warning]).not_to be_nil
        end
      end
    end