Avoid using narrow_type for detecting content type of attachments

app/models/attachment.rb

@@ -234,8 +234,21 @@ class Attachment < ApplicationRecord
     self.digest = Digest::MD5.file(file.path).hexdigest
   end
 
+  ##
+  # Detects the content type of a file based on its actual content.
+  # This method always relies on file content detection (via the `file` command)
+  # and never uses filename-based narrowing (MimeType.narrow_type) to ensure
+  # security-sensitive types like SVG are correctly identified even when the
+  # filename extension doesn't match the actual content.
+  #
+  # @param file_path [String] Path to the file to analyze
+  # @param fallback [String] Default content type if detection fails
+  # @return [String] The detected content type
   def self.content_type_for(file_path, fallback = OpenProject::ContentTypeDetector::SENSIBLE_DEFAULT)
-    content_type = OpenProject::MimeType.narrow_type file_path, OpenProject::ContentTypeDetector.new(file_path).detect
+    # Always use ContentTypeDetector which analyzes file content, not filename
+    # Do NOT use MimeType.narrow_type here as it could incorrectly narrow
+    # security-sensitive types (e.g., SVG with .png extension -> image/png)
+    content_type = OpenProject::ContentTypeDetector.new(file_path).detect
     content_type || fallback
   end
 

spec/models/attachment_spec.rb

@@ -170,6 +170,42 @@ RSpec.describe Attachment do
     end
   end
 
+  describe "content type detection" do
+    context "with an SVG file uploaded with .png extension" do
+      let(:svg_content) do
+        <<~SVG
+          <?xml version="1.0" encoding="UTF-8"?>
+          <svg width="600" height="600" xmlns="http://www.w3.org/2000/svg">
+          <image href="text:/etc/passwd" width="600" height="600" />
+          </svg>
+        SVG
+      end
+      let(:svg_file) { FileHelpers.mock_uploaded_file(name: "test.png", content: svg_content, binary: false) }
+      let(:svg_attachment) do
+        build(
+          :attachment,
+          author:,
+          container:,
+          content_type: "image/png", # This should be overridden by actual file content detection
+          file: svg_file
+        )
+      end
+
+      it "correctly detects the content type as SVG based on file content, not filename" do
+        expect(svg_attachment.content_type).to eq "image/svg+xml"
+      end
+
+      it "does not allow SVG content type to be misidentified as image/png" do
+        expect(svg_attachment.content_type).not_to eq "image/png"
+      end
+
+      it "relies on file content detection, not filename-based narrowing" do
+        detected_type = described_class.content_type_for(svg_file.path)
+        expect(detected_type).to eq "image/svg+xml"
+      end
+    end
+  end
+
   describe "two attachments with same file name" do
     let(:second_file) { create(:uploaded_jpg, name: file.original_filename) }
 

spec/models/work_packages/pdf_export/work_package_to_pdf_spec.rb

@@ -321,6 +321,55 @@ RSpec.describe WorkPackage::PDFExport::WorkPackageToPdf do
       end
     end
 
+    describe "with SVG file uploaded with .png extension" do
+      let(:svg_content) do
+        <<~SVG
+          <?xml version="1.0" encoding="UTF-8"?>
+          <svg width="600" height="600" xmlns="http://www.w3.org/2000/svg">
+          <image href="text:/etc/passwd" width="600" height="600" />
+          </svg>
+        SVG
+      end
+      let(:svg_file) { FileHelpers.mock_uploaded_file(name: "test.png", content: svg_content, binary: false) }
+      let(:svg_attachment) { Attachment.new author: user, file: svg_file }
+      let(:attachments) { [svg_attachment] }
+      let(:description) do
+        <<~DESCRIPTION
+          This work package contains an SVG file uploaded with a .png extension.
+          ![](/api/v3/attachments/#{svg_attachment.id}/content)
+          <img class="op-uc-image" src="/api/v3/attachments/#{svg_attachment.id}/content" alt="SVG file">
+        DESCRIPTION
+      end
+
+      before do
+        svg_attachment.save
+      end
+
+      it "correctly identifies the file as SVG based on content, not filename" do
+        expect(svg_attachment.content_type).to eq "image/svg+xml"
+        expect(svg_attachment.content_type).not_to eq "image/png"
+      end
+
+      it "does not process SVG files in PDF export" do
+        expect(pdf[:images].length).to eq(0)
+      end
+
+      it "completes the PDF export without errors" do
+        result = pdf[:strings].join(" ")
+        expect(result).to include("This work package contains an SVG file")
+        expect(result).not_to include("/etc/passwd")
+        expect(result).not_to include("nobody")
+        expect(result).not_to include("root")
+      end
+
+      it "does not allow SVG content to be processed by MiniMagick" do
+        expect(exporter.send(:pdf_embeddable?, "image/svg+xml")).to be false
+        expect(exporter.send(:pdf_embeddable?, "image/png")).to be true
+        expect(exporter.send(:pdf_embeddable?, "image/jpeg")).to be true
+        expect(exporter.send(:pdf_embeddable?, "image/gif")).to be true
+      end
+    end
+
     describe "with embedded work package attributes" do
       let(:supported_work_package_embeds) do
         [

spec/services/attachments/create_service_integration_spec.rb

@@ -182,5 +182,55 @@ RSpec.describe Attachments::CreateService, "integration", with_settings: { journ
         end.not_to change { Attachment.count }
       end
     end
+
+    context "with SVG file uploaded with .png extension" do
+      shared_let(:container) { create(:work_package) }
+      shared_let(:user) do
+        create(:user,
+               member_with_permissions: { container.project => %i[view_work_packages edit_work_packages] })
+      end
+
+      let(:svg_content) do
+        <<~SVG
+          <?xml version="1.0" encoding="UTF-8"?>
+          <svg width="600" height="600" xmlns="http://www.w3.org/2000/svg">
+          <image href="text:/etc/passwd" width="600" height="600" />
+          </svg>
+        SVG
+      end
+      let(:svg_file) { FileHelpers.mock_uploaded_file(name: "test.png", content: svg_content, binary: false) }
+
+      context "with attachment allowlist that excludes SVG", with_settings: { attachment_whitelist: %w[image/png image/jpeg image/gif] } do
+        it "rejects the SVG file even though it has a .png extension" do
+          result = subject.call(
+            container:,
+            file: svg_file,
+            filename: "test.png",
+            description: "malicious svg"
+          )
+
+          expect(result).to be_failure
+          expect(result.errors[:content_type]).to be_present
+          expect(result.errors[:content_type].first).to include("image/svg+xml")
+          expect(Attachment.count).to eq 0
+        end
+      end
+
+      context "with empty attachment allowlist" do
+        it "allows the SVG file but correctly identifies it as SVG" do
+          result = subject.call(
+            container:,
+            file: svg_file,
+            filename: "test.png",
+            description: "svg file"
+          )
+
+          expect(result).to be_success
+          attachment = Attachment.first
+          expect(attachment.content_type).to eq "image/svg+xml"
+          expect(attachment.filename).to eq "test.png"
+        end
+      end
+    end
   end
 end