From 42b835f0c018063a76c99f772bdcb8914d94b3e1 Mon Sep 17 00:00:00 2001 From: Dennis Schubert Date: Mon, 10 Jul 2023 00:16:49 +0200 Subject: [PATCH] Enforce an ImageMagick `policy.xml` for all pods. This fix was heavily inspired by Mastodon's fix for GHSA-9928-3cp5-93fm. So, thank you Cure53 for finding this issue, thank you Mozilla for paying Cure53 to look into it, and thanks for Mastodon for fixing it. --- Changelog.md | 4 ++++ config/imagemagick/policy.xml | 24 ++++++++++++++++++++++++ config/initializers/imagemagick.rb | 12 ++++++++++++ spec/fixtures/evil-image.ps.png | 12 ++++++++++++ spec/models/photo_spec.rb | 17 +++++++++++++++++ 5 files changed, 69 insertions(+) create mode 100644 config/imagemagick/policy.xml create mode 100644 config/initializers/imagemagick.rb create mode 100644 spec/fixtures/evil-image.ps.png diff --git a/Changelog.md b/Changelog.md index 61d76d7b5..76dcf06d2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +# 0.7.18.2 + +To avoid potential security issues, diaspora\* now makes sure that ImageMagick image processing always runs with a restricted `policy.xml`, regardless of the global system settings. + # 0.7.18.1 ## Bug fixes diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml new file mode 100644 index 000000000..50c92e6ef --- /dev/null +++ b/config/imagemagick/policy.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/config/initializers/imagemagick.rb b/config/initializers/imagemagick.rb new file mode 100644 index 000000000..df7acfa28 --- /dev/null +++ b/config/initializers/imagemagick.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# This is based on Mastodon doing the same, see +# https://github.com/mastodon/mastodon/blob/610cf6c3713e414995ea1a57110db400ccb88dd2/config/initializers/paperclip.rb#L157-L162 +# At the time of writing, Mastodon is also licensed under the AGPL, see https://github.com/mastodon/mastodon/blob/610cf6c3713e414995ea1a57110db400ccb88dd2/LICENSE +# so the following snippet is Copyright (C) 2016-2022 Eugen Rochko & other Mastodon contributors. +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 +# end of Mastodon snippet diff --git a/spec/fixtures/evil-image.ps.png b/spec/fixtures/evil-image.ps.png new file mode 100644 index 000000000..76c563dad --- /dev/null +++ b/spec/fixtures/evil-image.ps.png @@ -0,0 +1,12 @@ +%! +%% ohno + +/Times-Roman findfont +12 scalefont +setfont + +newpath +100 200 moveto +(ohno) show + +showpage diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 225d609a8..ea51eee99 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -264,4 +264,21 @@ describe Photo, :type => :model do end end end + + context "with a maliciously crafted image" do + let(:base_path) { File.dirname(__FILE__) } + let(:public_path) { File.join(base_path, "../../public/") } + let(:evil_image) { File.open(File.join(base_path, "..", "fixtures", "evil-image.ps.png")) } + + it "fails to process a PostScript file camouflaged as a PNG" do + photo = bob.build_post(:photo, user_file: evil_image, to: @aspect.id) + + expect { + with_carrierwave_processing do + photo.unprocessed_image.store! evil_image + photo.save + end + }.to raise_error(CarrierWave::ProcessingError) + end + end end