From 5ddae512857eb143ff91741f4a35c186fac1036e Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 6 Jul 2023 15:03:33 +0200 Subject: Merge pull request from GHSA-ccm4-vgcc-73hp * Tighten allowed HTML in oEmbed-based preview cards * Sanitize preview cards at render time * Add `sandbox` attribute to preview card iframes --- lib/sanitize_ext/sanitize_config.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'lib') diff --git a/lib/sanitize_ext/sanitize_config.rb b/lib/sanitize_ext/sanitize_config.rb index 82834fd68..85a6edcfe 100644 --- a/lib/sanitize_ext/sanitize_config.rb +++ b/lib/sanitize_ext/sanitize_config.rb @@ -100,26 +100,26 @@ class Sanitize ] ) - MASTODON_OEMBED ||= freeze_config merge( - RELAXED, - elements: RELAXED[:elements] + %w(audio embed iframe source video), + MASTODON_OEMBED ||= freeze_config( + elements: %w(audio embed iframe source video), - attributes: merge( - RELAXED[:attributes], + attributes: { 'audio' => %w(controls), 'embed' => %w(height src type width), 'iframe' => %w(allowfullscreen frameborder height scrolling src width), 'source' => %w(src type), 'video' => %w(controls height loop width), - 'div' => [:data] - ), + }, - protocols: merge( - RELAXED[:protocols], + protocols: { 'embed' => { 'src' => HTTP_PROTOCOLS }, 'iframe' => { 'src' => HTTP_PROTOCOLS }, - 'source' => { 'src' => HTTP_PROTOCOLS } - ) + 'source' => { 'src' => HTTP_PROTOCOLS }, + }, + + add_attributes: { + 'iframe' => { 'sandbox' => 'allow-scripts allow-same-origin allow-popups allow-popups-to-escape-sandbox allow-forms' }, + } ) LINK_REL_TRANSFORMER = lambda do |env| -- cgit From c58ab80b976410b51bc5520b00a98dcdc256dee3 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 6 Jul 2023 15:05:05 +0200 Subject: Merge pull request from GHSA-9928-3cp5-93fm * Fix attachments getting processed despite failing content-type validation * Add a restrictive ImageMagick security policy tailored for Mastodon * Fix misdetection of MP3 files with large cover art * Reject unprocessable audio/video files instead of keeping them unchanged --- app/models/concerns/attachmentable.rb | 5 ++-- config/application.rb | 1 + config/imagemagick/policy.xml | 27 +++++++++++++++++++++ config/initializers/paperclip.rb | 7 ++++++ .../media_type_spoof_detector_extensions.rb | 22 +++++++++++++++++ lib/paperclip/transcoder.rb | 5 +--- spec/fixtures/files/boop.mp3 | Bin 0 -> 7841601 bytes spec/models/media_attachment_spec.rb | 20 +++++++++++++++ 8 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 config/imagemagick/policy.xml create mode 100644 lib/paperclip/media_type_spoof_detector_extensions.rb create mode 100644 spec/fixtures/files/boop.mp3 (limited to 'lib') 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 @@ + + + + + + + + + + + + + + + + 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 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 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')) } -- cgit