Skip to content

Commit

Permalink
Prevent remote shell execution in #apply
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
janko committed Mar 1, 2022
1 parent 183c058 commit 038e457
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
8 changes: 4 additions & 4 deletions lib/image_processing/chainable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions test/pipeline_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 038e457

Please sign in to comment.