Skip to content

Commit

Permalink
Fix Code Injection vulnerability in CarrierWave::RMagick
Browse files Browse the repository at this point in the history
  • Loading branch information
mshibuya committed Feb 8, 2021
1 parent 91714ad commit eb9346d
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
12 changes: 9 additions & 3 deletions lib/carrierwave/processing/rmagick.rb
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,15 @@ def manipulate!(options={}, &block)

def create_info_block(options)
return nil unless options
assignments = options.map { |k, v| "self.#{k} = #{v}" }
code = "lambda { |img| " + assignments.join(";") + "}"
eval code
proc do |img|
options.each do |k, v|
if v.is_a?(String) && (matches = v.match(/^["'](.+)["']/))
ActiveSupport::Deprecation.warn "Passing quoted strings like #{v} to #manipulate! is deprecated, pass them without quoting."
v = matches[1]
end
img.public_send(:"#{k}=", v)
end
end
end

def destroy_image(image)
Expand Down
35 changes: 34 additions & 1 deletion spec/processing/rmagick_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,42 @@

instance.manipulate! :read => {
:density => 10,
:size => %{"200x200"}
:size => "200x200"
}
end

it 'shows deprecation but still accepts strings enclosed with double quotes' do
expect_any_instance_of(::Magick::Image::Info).to receive(:size=).once.with("200x200")
expect(ActiveSupport::Deprecation).to receive(:warn).with(any_args)
instance.manipulate! :read => {:size => %{"200x200"}}
end

it 'shows deprecation but still accepts strings enclosed with single quotes' do
expect_any_instance_of(::Magick::Image::Info).to receive(:size=).once.with("200x200")
expect(ActiveSupport::Deprecation).to receive(:warn).with(any_args)
instance.manipulate! :read => {:size => %{'200x200'}}
end

it 'does not allow arbitrary code execution' do
expect_any_instance_of(Kernel).not_to receive(:puts)
expect do
instance.manipulate! :read => {
:density => "1 }; raise; {"
}
end.to raise_error ArgumentError, /invalid density geometry/
end

it 'does not allow invocation of non-public methods' do
module Kernel
private
def foo=(value); raise; end
end
expect do
instance.manipulate! :read => {
:foo => "1"
}
end.to raise_error NoMethodError, /private method `foo=' called/
end
end

describe "#width and #height" do
Expand Down

0 comments on commit eb9346d

Please sign in to comment.