diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb
index 9cafedc209021bdb682dece760f882220a771c48..f93ee4c919a2289df0d6d043fa601e193b9af64e 100644
--- a/app/models/concerns/attachmentable.rb
+++ b/app/models/concerns/attachmentable.rb
@@ -22,15 +22,14 @@ module Attachmentable
included do
def self.has_attached_file(name, options = {}) # rubocop:disable Naming/PredicateName
- options = { validate_media_type: false }.merge(options)
super(name, options)
- send(:"before_#{name}_post_process") do
+
+ send(:"before_#{name}_validate") do
attachment = send(name)
check_image_dimension(attachment)
set_file_content_type(attachment)
obfuscate_file_name(attachment)
set_file_extension(attachment)
- Paperclip::Validators::MediaTypeSpoofDetectionValidator.new(attributes: [name]).validate(self)
end
end
end
diff --git a/config/application.rb b/config/application.rb
index d3c99baa1273db84e1222761b3ebabc611b7fb33..8c4ec27e7f3d88579d8d0c849f3fccd10893f145 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -28,6 +28,7 @@ require_relative '../lib/paperclip/url_generator_extensions'
require_relative '../lib/paperclip/attachment_extensions'
require_relative '../lib/paperclip/lazy_thumbnail'
require_relative '../lib/paperclip/gif_transcoder'
+require_relative '../lib/paperclip/media_type_spoof_detector_extensions'
require_relative '../lib/paperclip/transcoder'
require_relative '../lib/paperclip/type_corrector'
require_relative '../lib/paperclip/response_with_limit_adapter'
diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml
new file mode 100644
index 0000000000000000000000000000000000000000..1052476b319eb33487c22e96be383985cb63657a
--- /dev/null
+++ b/config/imagemagick/policy.xml
@@ -0,0 +1,27 @@
+<policymap>
+ <!-- Set some basic system resource limits -->
+ <policy domain="resource" name="time" value="60" />
+
+ <policy domain="module" rights="none" pattern="URL" />
+
+ <policy domain="filter" rights="none" pattern="*" />
+
+ <!--
+ Ideally, we would restrict ImageMagick to only accessing its own
+ disk-backed pixel cache as well as Mastodon-created Tempfiles.
+
+ However, those paths depend on the operating system and environment
+ variables, so they can only be known at runtime.
+
+ Furthermore, those paths are not necessarily shared across Mastodon
+ processes, so even creating a policy.xml at runtime is impractical.
+
+ For the time being, only disable indirect reads.
+ -->
+ <policy domain="path" rights="none" pattern="@*" />
+
+ <!-- Disallow any coder by default, and only enable ones required by Mastodon -->
+ <policy domain="coder" rights="none" pattern="*" />
+ <policy domain="coder" rights="read | write" pattern="{PNG,JPEG,GIF,HEIC,WEBP}" />
+ <policy domain="coder" rights="write" pattern="{HISTOGRAM,RGB,INFO}" />
+</policymap>
diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb
index 093d2ba9ae2bb38208a097964ddcce0796a53683..f2da410dbe947df420b076ae2e0aefafadec2625 100644
--- a/config/initializers/paperclip.rb
+++ b/config/initializers/paperclip.rb
@@ -153,3 +153,10 @@ unless defined?(Seahorse)
end
end
end
+
+# Set our ImageMagick security policy, but allow admins to override it
+ENV['MAGICK_CONFIGURE_PATH'] = begin
+ imagemagick_config_paths = ENV.fetch('MAGICK_CONFIGURE_PATH', '').split(File::PATH_SEPARATOR)
+ imagemagick_config_paths << Rails.root.join('config', 'imagemagick').expand_path.to_s
+ imagemagick_config_paths.join(File::PATH_SEPARATOR)
+end
diff --git a/lib/paperclip/media_type_spoof_detector_extensions.rb b/lib/paperclip/media_type_spoof_detector_extensions.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a406ef312fd3c7cc920b78e4337016f8e839ba21
--- /dev/null
+++ b/lib/paperclip/media_type_spoof_detector_extensions.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Paperclip
+ module MediaTypeSpoofDetectorExtensions
+ def calculated_content_type
+ return @calculated_content_type if defined?(@calculated_content_type)
+
+ @calculated_content_type = type_from_file_command.chomp
+
+ # The `file` command fails to recognize some MP3 files as such
+ @calculated_content_type = type_from_marcel if @calculated_content_type == 'application/octet-stream' && type_from_marcel == 'audio/mpeg'
+ @calculated_content_type
+ end
+
+ def type_from_marcel
+ @type_from_marcel ||= Marcel::MimeType.for Pathname.new(@file.path),
+ name: @file.path
+ end
+ end
+end
+
+Paperclip::MediaTypeSpoofDetector.prepend(Paperclip::MediaTypeSpoofDetectorExtensions)
diff --git a/lib/paperclip/transcoder.rb b/lib/paperclip/transcoder.rb
index afd9f58ff695c00b39e57b1485068e097679f7a1..be40b4924107aa8face452c4d6ad4264e416b80e 100644
--- a/lib/paperclip/transcoder.rb
+++ b/lib/paperclip/transcoder.rb
@@ -19,10 +19,7 @@ module Paperclip
def make
metadata = VideoMetadataExtractor.new(@file.path)
- unless metadata.valid?
- Paperclip.log("Unsupported file #{@file.path}")
- return File.open(@file.path)
- end
+ raise Paperclip::Error, "Error while transcoding #{@file.path}: unsupported file" unless metadata.valid?
update_attachment_type(metadata)
update_options_from_metadata(metadata)
diff --git a/spec/fixtures/files/boop.mp3 b/spec/fixtures/files/boop.mp3
new file mode 100644
index 0000000000000000000000000000000000000000..ba106a3a32d414263d3c1150d70b4ece2ea1e9d4
Binary files /dev/null and b/spec/fixtures/files/boop.mp3 differ
diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb
index 2dfc6cf9252237c948ec2de03023b4cac2a43735..90e4f2f47b727d897eebbf6327adba58702bc9dd 100644
--- a/spec/models/media_attachment_spec.rb
+++ b/spec/models/media_attachment_spec.rb
@@ -152,6 +152,26 @@ RSpec.describe MediaAttachment, paperclip_processing: true do
end
end
+ describe 'mp3 with large cover art' do
+ let(:media) { described_class.create(account: Fabricate(:account), file: attachment_fixture('boop.mp3')) }
+
+ it 'detects it as an audio file' do
+ expect(media.type).to eq 'audio'
+ end
+
+ it 'sets meta for the duration' do
+ expect(media.file.meta['original']['duration']).to be_within(0.05).of(0.235102)
+ end
+
+ it 'extracts thumbnail' do
+ expect(media.thumbnail.present?).to be true
+ end
+
+ it 'gives the file a random name' do
+ expect(media.file_file_name).to_not eq 'boop.mp3'
+ end
+ end
+
describe 'jpeg' do
let(:media) { described_class.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) }