Run ImageMagick under a policy that travels with the project

2 files changed, 60 insertions, 4 deletions

diff --git a/app/models/concerns/file_attachment.rb b/app/models/concerns/file_attachment.rb
index 5e0803fd..fd0cabed 100644
--- a/app/models/concerns/file_attachment.rb
+++ b/app/models/concerns/file_attachment.rb
@@ -29,6 +29,14 @@ module FileAttachment
     large:    { args: ["-resize", "1600x1600>"] }
   }.freeze
 
+  # Prepended to ImageMagick's configuration search path on every
+  # invocation, so config/imagemagick/policy.xml is read before the
+  # installed one. Per-invocation rather than in the environment, so it
+  # cannot leak into anything else on the host.
+  MAGICK_ENV = {
+    "MAGICK_CONFIGURE_PATH" => Rails.root.join("config", "imagemagick").to_s
+  }.freeze
+
   IMAGE_CONTENT_TYPES    = %w[image/jpeg image/gif image/png image/webp].freeze
   VECTOR_CONTENT_TYPES   = %w[image/svg+xml].freeze
   RASTERIZED_CONTENT_TYPES = %w[application/pdf].freeze
@@ -152,7 +160,7 @@ module FileAttachment
     STYLES.each do |style, options|
       dest_path = file_path(style)
       FileUtils.mkdir_p(File.dirname(dest_path))
-      system("magick", original_path, *extra_args, *options[:args], dest_path)
+      system(MAGICK_ENV, "magick", original_path, *extra_args, *options[:args], dest_path)
     end
   end
 
@@ -173,13 +181,13 @@ module FileAttachment
     FileUtils.mkdir_p(File.dirname(dest_path))
 
     if og_full_bleed?(original_path)
-      system("magick", "#{original_path}[0]",
+      system(MAGICK_ENV, "magick", "#{original_path}[0]",
              "-resize", "#{OG_WIDTH}x#{OG_HEIGHT}^",
              "-gravity", "center", "-extent", "#{OG_WIDTH}x#{OG_HEIGHT}",
              "-background", OG_GROUND, "-alpha", "remove", "-alpha", "off",
              *og_output_args, dest_path)
     else
-      system(*og_template_command(dest_path))
+      system(MAGICK_ENV, *og_template_command(dest_path))
     end
   end
 
@@ -206,7 +214,7 @@ module FileAttachment
   end
 
   def source_dimensions(path)
-    out, status = Open3.capture2("magick", "identify", "-format", "%w %h", "#{path}[0]")
+    out, status = Open3.capture2(MAGICK_ENV, "magick", "identify", "-format", "%w %h", "#{path}[0]")
     return nil unless status.success?
 
     width, height = out.split.map(&:to_i)
diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml
new file mode 100644
index 00000000..6106aa0b
--- /dev/null
+++ b/config/imagemagick/policy.xml
@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Read because FileAttachment sets MAGICK_CONFIGURE_PATH to this directory
+  on every magick invocation. ImageMagick prepends that path to its search
+  list rather than replacing it, so /usr/local/etc/ImageMagick-7/policy.xml
+  is still read afterwards and any hardening the port gains still applies.
+  Rules are processed in order, so these win.
+
+  Coders kept readable and why:
+    PDF, PS   the rasterised branch and the 150dpi social card
+    SVG, MSVG the vector social card
+    JPEG PNG GIF WEBP  the four accepted image types
+    LABEL     used by internal operations
+-->
+<policymap>
+  <!-- Variants are generated synchronously on upload, so an oversized or
+       slow source blocks a request thread. A decompression bomb needs no
+       vulnerability at all. -->
+  <policy domain="resource" name="memory" value="256MiB"/>
+  <policy domain="resource" name="map" value="512MiB"/>
+  <policy domain="resource" name="area" value="128MP"/>
+  <policy domain="resource" name="disk" value="1GiB"/>
+  <policy domain="resource" name="width" value="16KP"/>
+  <policy domain="resource" name="height" value="16KP"/>
+  <policy domain="resource" name="list-length" value="64"/>
+  <policy domain="resource" name="time" value="120"/>
+
+  <!-- @file means "read this path as the argument", and - is stdin. -->
+  <policy domain="path" rights="none" pattern="@*"/>
+  <policy domain="path" rights="none" pattern="-"/>
+
+  <!-- ImageMagick picks its decoder from the file's bytes, not from the
+       declared content type, so an upload announced as image/jpeg is
+       decoded as MVG if that is what it contains. These are the coders
+       that turn that into a file read or a fetch. -->
+  <policy domain="coder" rights="none" pattern="MVG"/>
+  <policy domain="coder" rights="none" pattern="MSL"/>
+  <policy domain="coder" rights="none" pattern="URL"/>
+  <policy domain="coder" rights="none" pattern="HTTP"/>
+  <policy domain="coder" rights="none" pattern="HTTPS"/>
+  <policy domain="coder" rights="none" pattern="FTP"/>
+  <policy domain="coder" rights="none" pattern="EPHEMERAL"/>
+  <policy domain="coder" rights="none" pattern="SHOW"/>
+  <policy domain="coder" rights="none" pattern="WIN"/>
+  <policy domain="coder" rights="none" pattern="PLT"/>
+  <policy domain="coder" rights="none" pattern="XPS"/>
+  <policy domain="coder" rights="none" pattern="TEXT"/>
+</policymap>