diff options
| -rw-r--r-- | app/models/concerns/attachmentable.rb | 5 | ||||
| -rw-r--r-- | config/application.rb | 1 | ||||
| -rw-r--r-- | config/imagemagick/policy.xml | 27 | ||||
| -rw-r--r-- | config/initializers/paperclip.rb | 7 | ||||
| -rw-r--r-- | lib/paperclip/media_type_spoof_detector_extensions.rb | 22 | ||||
| -rw-r--r-- | lib/paperclip/transcoder.rb | 5 | ||||
| -rw-r--r-- | spec/fixtures/files/boop.mp3 | bin | 0 -> 7841601 bytes | |||
| -rw-r--r-- | spec/models/media_attachment_spec.rb | 20 |
8 files changed, 80 insertions, 7 deletions
diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index d44c22438..28591ab72 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 f72cc8e11..4a440c6f2 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 000000000..1052476b3 --- /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 bd37f6709..ca600346a 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -161,3 +161,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 000000000..a406ef312 --- /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 b3b55f82f..f4768aa60 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 000000000..ba106a3a3 --- /dev/null +++ b/spec/fixtures/files/boop.mp3 Binary files differdiff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 63edfc152..1193924fd 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -152,6 +152,26 @@ RSpec.describe MediaAttachment, type: :model 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) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) } |