From eb9346df598a758a5f8c4a338852982fd7f8f6b8 Mon Sep 17 00:00:00 2001 From: "M.Shibuya" Date: Sun, 7 Feb 2021 15:26:22 +0900 Subject: [PATCH] Fix Code Injection vulnerability in CarrierWave::RMagick Refs. https://github.com/carrierwaveuploader/carrierwave/security/advisories/GHSA-cf3w-g86h-35x4 --- lib/carrierwave/processing/rmagick.rb | 12 ++++++--- spec/processing/rmagick_spec.rb | 35 ++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/lib/carrierwave/processing/rmagick.rb b/lib/carrierwave/processing/rmagick.rb index 424ef225c..743a39434 100644 --- a/lib/carrierwave/processing/rmagick.rb +++ b/lib/carrierwave/processing/rmagick.rb @@ -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) diff --git a/spec/processing/rmagick_spec.rb b/spec/processing/rmagick_spec.rb index 2c5d95fde..8325975df 100644 --- a/spec/processing/rmagick_spec.rb +++ b/spec/processing/rmagick_spec.rb @@ -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