From 00676e23d7f4beac12beddee6f2486b686fb7e46 Mon Sep 17 00:00:00 2001
From: Mitsuhiro Shibuya <mit.shibuya@gmail.com>
Date: Wed, 20 Mar 2024 14:10:19 +0900
Subject: [PATCH] Fix Content-Type allowlist bypass vulnerability remained

Refs. https://github.com/carrierwaveuploader/carrierwave/security/advisories/GHSA-vfmv-jfc5-pjjw
---
 lib/carrierwave/sanitized_file.rb            |  2 +-
 spec/sanitized_file_spec.rb                  | 27 ++++++++++++++++++++
 spec/uploader/content_type_allowlist_spec.rb | 16 ------------
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/lib/carrierwave/sanitized_file.rb b/lib/carrierwave/sanitized_file.rb
index 2419d3668..f8772e337 100644
--- a/lib/carrierwave/sanitized_file.rb
+++ b/lib/carrierwave/sanitized_file.rb
@@ -307,7 +307,7 @@ def sanitize(name)
     def declared_content_type
       @declared_content_type ||
         if @file.respond_to?(:content_type) && @file.content_type
-          @file.content_type.to_s.chomp
+          Marcel::MimeType.for(declared_type: @file.content_type.to_s.chomp)
         end
     end
 
diff --git a/spec/sanitized_file_spec.rb b/spec/sanitized_file_spec.rb
index ea6cd943d..71b288d00 100644
--- a/spec/sanitized_file_spec.rb
+++ b/spec/sanitized_file_spec.rb
@@ -306,6 +306,33 @@
 
       expect { sanitized_file.content_type }.not_to raise_error
     end
+
+    it "uses the first one when multiple mime types are given using a semicolon" do
+      file = File.open(file_path("bork.txt"))
+      allow(file).to receive(:content_type) { 'image/png; text/html' }
+
+      sanitized_file = CarrierWave::SanitizedFile.new(file)
+
+      expect(sanitized_file.content_type).to eq("image/png")
+    end
+
+    it "uses the first one when multiple mime types are given using a comma" do
+      file = File.open(file_path("bork.txt"))
+      allow(file).to receive(:content_type) { 'image/png, text/html' }
+
+      sanitized_file = CarrierWave::SanitizedFile.new(file)
+
+      expect(sanitized_file.content_type).to eq("image/png")
+    end
+
+    it "drops content type parameters" do
+      file = File.open(file_path("bork.txt"))
+      allow(file).to receive(:content_type) { 'text/html; charset=utf-8' }
+
+      sanitized_file = CarrierWave::SanitizedFile.new(file)
+
+      expect(sanitized_file.content_type).to eq("text/html")
+    end
   end
 
   describe "#content_type=" do
diff --git a/spec/uploader/content_type_allowlist_spec.rb b/spec/uploader/content_type_allowlist_spec.rb
index e38def1c0..fd25a4890 100644
--- a/spec/uploader/content_type_allowlist_spec.rb
+++ b/spec/uploader/content_type_allowlist_spec.rb
@@ -87,22 +87,6 @@
           expect { uploader.cache!(bork_file) }.to raise_error(CarrierWave::IntegrityError)
         end
       end
-
-      context "when the allowlist contains charset" do
-        before do
-          allow(uploader).to receive(:content_type_allowlist).and_return(%r{text/plain;\s*charset=utf-8})
-        end
-
-        it "accepts the content with allowed charset" do
-          allow(bork_file).to receive(:content_type).and_return('text/plain; charset=utf-8')
-          expect { uploader.cache!(bork_file) }.not_to raise_error
-        end
-
-        it "rejects the content without charset" do
-          allow(bork_file).to receive(:content_type).and_return('text/plain')
-          expect { uploader.cache!(bork_file) }.to raise_error(CarrierWave::IntegrityError)
-        end
-      end
     end
 
     context "when there is a whitelist" do