From 038e4574e8f4f4b636a62394e09983c71980dada Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Janko=20Marohni=C4=87?= <janko.marohnic@gmail.com>
Date: Tue, 1 Mar 2022 22:09:02 +0100
Subject: [PATCH] Prevent remote shell execution in `#apply`

If the operations are coming from user input, this could allow the user
to execute arbitrary shell commands via `Kernel#system` and
`Kernel#spawn`:

  ImageProcessing::Vips.apply({ system: "echo something" })

We prevent this by using `#public_send` instead of `#send`, which goes
to method missing instead of calling private methods, which include
`Kernel#system` and `Kernel#spawn`.
---
 CHANGELOG.md                      |  4 ++++
 lib/image_processing/chainable.rb |  8 ++++----
 test/pipeline_test.rb             | 15 +++++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ad8cb83..8f1807a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## HEAD
+
+* Prevent remote shell execution when using `#apply` with operations coming from user input (@janko)
+
 ## 1.12.1 (2020-11-06)
 
 * Fix format fallback for files ending with a dot on Ruby 2.7+ (@coding-chimp)
diff --git a/lib/image_processing/chainable.rb b/lib/image_processing/chainable.rb
index 53040dc..06311de 100644
--- a/lib/image_processing/chainable.rb
+++ b/lib/image_processing/chainable.rb
@@ -34,13 +34,13 @@ def instrumenter(&block)
     def apply(operations)
       operations.inject(self) do |builder, (name, argument)|
         if argument == true || argument == nil
-          builder.send(name)
+          builder.public_send(name)
         elsif argument.is_a?(Array)
-          builder.send(name, *argument)
+          builder.public_send(name, *argument)
         elsif argument.is_a?(Hash)
-          builder.send(name, **argument)
+          builder.public_send(name, **argument)
         else
-          builder.send(name, argument)
+          builder.public_send(name, argument)
         end
       end
     end
diff --git a/test/pipeline_test.rb b/test/pipeline_test.rb
index 9c21a45..d3d7162 100644
--- a/test/pipeline_test.rb
+++ b/test/pipeline_test.rb
@@ -258,4 +258,19 @@
       ImageProcessing::Vips.valid?(@portrait)
     end
   end
+
+  it "doesn't allow making system calls" do
+    ImageProcessing::Vips.source(@portrait).apply(system: "touch foo.txt")
+    refute File.exist?("foo.txt")
+
+    assert_raises Vips::Error do
+      ImageProcessing::Vips.source(@portrait).spawn("touch foo.txt").call
+    end
+    refute File.exist?("foo.txt")
+
+    assert_raises MiniMagick::Error do
+      ImageProcessing::MiniMagick.source(@portrait).spawn("touch foo.txt").call
+    end
+    refute File.exist?("foo.txt")
+  end
 end