From 4edd6d14816f215521b8279925c04672e6e3777d Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Mon, 27 Mar 2023 14:30:00 +0200 Subject: [PATCH 1/2] Remove bind method from Safemode::Scope The bind method could be used to execute arbitrary system commands by leveraging that things passed into it were getting eval'd without any escaping. This should remove the issue completely by making bind unavailable. --- lib/safemode.rb | 7 ++++--- lib/safemode/scope.rb | 12 ++++++------ test/test_safemode_eval.rb | 4 ++++ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/safemode.rb b/lib/safemode.rb index bbefa17..4ca0cd7 100644 --- a/lib/safemode.rb +++ b/lib/safemode.rb @@ -40,15 +40,16 @@ def find_jail_class(klass) class Box def initialize(delegate = nil, delegate_methods = [], filename = nil, line = nil) - @scope = Scope.new(delegate, delegate_methods) + @delegate = delegate + @delegate_methods = delegate_methods @filename = filename @line = line end def eval(code, assigns = {}, locals = {}, &block) code = Parser.jail(code) - binding = @scope.bind(assigns, locals, &block) - result = Kernel.eval(code, binding, @filename || __FILE__, @line || __LINE__) + scope = Scope.new(@delegate, @delegate_methods, instance_vars: assigns, locals: locals, &block) + result = Kernel.eval(code, scope.get_binding, @filename || __FILE__, @line || __LINE__) end def output diff --git a/lib/safemode/scope.rb b/lib/safemode/scope.rb index 1c3ab6d..93daa25 100644 --- a/lib/safemode/scope.rb +++ b/lib/safemode/scope.rb @@ -1,17 +1,17 @@ module Safemode class Scope < Blankslate - def initialize(delegate = nil, delegate_methods = []) + def initialize(delegate = nil, delegate_methods = [], instance_vars: {}, locals: {}, &block) @delegate = delegate @delegate_methods = delegate_methods - @locals = {} - end - - def bind(instance_vars = {}, locals = {}, &block) @locals = symbolize_keys(locals) # why can't I just pull them to local scope in the same way like instance_vars? instance_vars = symbolize_keys(instance_vars) instance_vars.each {|key, obj| eval "@#{key} = instance_vars[:#{key}]" } @_safemode_output = '' - binding + @binding = binding + end + + def get_binding + @binding end def to_jail diff --git a/test/test_safemode_eval.rb b/test/test_safemode_eval.rb index 051360b..83fbc82 100644 --- a/test/test_safemode_eval.rb +++ b/test/test_safemode_eval.rb @@ -80,6 +80,10 @@ def test_interpolated_xstr_should_raise_security assert_raise_security '"#{`ls -a`}"' end + def test_should_not_allow_access_to_bind + assert_raise_security "self.bind('an arg')" + end + TestHelper.no_method_error_raising_calls.each do |call| call.gsub!('"', '\\\\"') class_eval %Q( From 69d703f084f75c27f216e4249db3e49ecf6a2bf3 Mon Sep 17 00:00:00 2001 From: Adam Ruzicka Date: Wed, 3 May 2023 09:03:06 +0000 Subject: [PATCH 2/2] Fix Box#output --- lib/safemode.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/safemode.rb b/lib/safemode.rb index 4ca0cd7..a227891 100644 --- a/lib/safemode.rb +++ b/lib/safemode.rb @@ -48,12 +48,12 @@ def initialize(delegate = nil, delegate_methods = [], filename = nil, line = nil def eval(code, assigns = {}, locals = {}, &block) code = Parser.jail(code) - scope = Scope.new(@delegate, @delegate_methods, instance_vars: assigns, locals: locals, &block) - result = Kernel.eval(code, scope.get_binding, @filename || __FILE__, @line || __LINE__) + @scope = Scope.new(@delegate, @delegate_methods, instance_vars: assigns, locals: locals, &block) + Kernel.eval(code, @scope.get_binding, @filename || __FILE__, @line || __LINE__) end - + def output @scope.output - end + end end end