diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 52438016546..0679e6caa55 100644
--- a/app/models/attachment.rb
+++ b/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
diff --git a/spec/models/attachment_spec.rb b/spec/models/attachment_spec.rb
index 6b6b2064b8e..de4a9796290 100644
--- a/spec/models/attachment_spec.rb
+++ b/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
+
+
+ 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) }
diff --git a/spec/models/work_packages/pdf_export/work_package_to_pdf_spec.rb b/spec/models/work_packages/pdf_export/work_package_to_pdf_spec.rb
index cc78b6bd6af..06334fed46c 100644
--- a/spec/models/work_packages/pdf_export/work_package_to_pdf_spec.rb
+++ b/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
+
+
+ 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.
+ 
+
+ 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
[
diff --git a/spec/services/attachments/create_service_integration_spec.rb b/spec/services/attachments/create_service_integration_spec.rb
index 7cab543013e..59a0d18a5f7 100644
--- a/spec/services/attachments/create_service_integration_spec.rb
+++ b/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
+
+
+ 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