diff --git a/lib/safemode/parser.rb b/lib/safemode/parser.rb index 0d298d6..c731bda 100644 --- a/lib/safemode/parser.rb +++ b/lib/safemode/parser.rb @@ -36,12 +36,12 @@ def jail(str, parentheses = false, safe_call: false) # split up #process_call. see below ... def process_call(exp, safe_call = false) - exp.shift # remove ":call" symbol - receiver = jail(process_call_receiver(exp), safe_call: safe_call) - name = exp.shift - args = process_call_args(exp) + _, recv, name, *args = exp - process_call_code(receiver, name, args, safe_call) + receiver = jail(process_call_receiver(recv), safe_call: safe_call) + arguments = process_call_args(name, args) + + process_call_code(receiver, name, arguments, safe_call) end def process_fcall(exp) @@ -143,26 +143,35 @@ def raise_security_error(type, info) # split up Ruby2Ruby#process_call monster method so we can hook into it # in a more readable manner - def process_call_receiver(exp) - receiver_node_type = exp.first.nil? ? nil : exp.first.first - receiver = process exp.shift - receiver = "(#{receiver})" if - Ruby2Ruby::ASSIGN_NODES.include? receiver_node_type + def process_call_receiver(recv) + receiver_node_type = recv && recv.sexp_type + receiver = process recv + receiver = "(#{receiver})" if ASSIGN_NODES.include? receiver_node_type receiver end - def process_call_args(exp) - args = [] - while not exp.empty? do - args_exp = exp.shift - if args_exp && args_exp.first == :array # FIX - processed = "#{process(args_exp)[1..-2]}" - else - processed = process args_exp - end - args << processed unless (processed.nil? or processed.empty?) + def process_call_args(name, args) + in_context :arglist do + max = args.size - 1 + args = args.map.with_index { |arg, i| + arg_type = arg.sexp_type + is_empty_hash = arg == s(:hash) + arg = process arg + + next if arg.empty? + + strip_hash = (arg_type == :hash and + not BINARY.include? name and + not is_empty_hash and + (i == max or args[i + 1].sexp_type == :splat)) + wrap_arg = Ruby2Ruby::ASSIGN_NODES.include? arg_type + + arg = arg[2..-3] if strip_hash + arg = "(#{arg})" if wrap_arg + + arg + }.compact end - args end def process_call_code(receiver, name, args, safe_call) diff --git a/test/test_safemode_eval.rb b/test/test_safemode_eval.rb index c9a23d9..832fdb3 100644 --- a/test/test_safemode_eval.rb +++ b/test/test_safemode_eval.rb @@ -88,6 +88,16 @@ def test_should_not_allow_access_to_bind assert_raise_security "self.bind('an arg')" end + def test_sending_of_kwargs_works + assert @box.eval("@article.method_with_kwargs(a_keyword: true)", @assigns) + end + + def test_sending_to_method_missing + assert_raise_with_message(Safemode::NoMethodError, /#no_such_method/) do + @box.eval("@article.no_such_method('arg', key: 'value')", @assigns) + end + end + TestHelper.no_method_error_raising_calls.each do |call| call.gsub!('"', '\\\\"') class_eval %Q( diff --git a/test/test_safemode_parser.rb b/test/test_safemode_parser.rb index 03697d9..f049230 100644 --- a/test/test_safemode_parser.rb +++ b/test/test_safemode_parser.rb @@ -25,13 +25,31 @@ def test_ternary_should_be_usable_for_erb assert_jailed "if true then\n 1\n else\n2\nend", "true ? 1 : 2" end + def test_call_with_shorthand + unsafe = <<~UNSAFE + a_keyword = true + @article.method_with_kwargs(a_keyword:) + UNSAFE + jailed = <<~JAILED + a_keyword = true + @article.to_jail.method_with_kwargs(a_keyword:) + JAILED + assert_jailed jailed, unsafe + end + + def test_call_with_complex_args + unsafe = "@article.method_with_kwargs('positional', a_keyword: true, **kwargs)" + jailed = "@article.to_jail.method_with_kwargs(\"positional\", :a_keyword => true, **to_jail.kwargs)" + assert_jailed jailed, unsafe + end + def test_safe_call_simple assert_jailed '@article&.to_jail&.method', '@article&.method' end def test_safe_call_with_complex_args - unsafe = "@article&.method_with_kwargs('positional')" - jailed = "@article&.to_jail&.method_with_kwargs(\"positional\")" + unsafe = "@article&.method_with_kwargs('positional', a_keyword: true, **kwargs)" + jailed = "@article&.to_jail&.method_with_kwargs(\"positional\", :a_keyword => true, **to_jail.kwargs)" assert_jailed jailed, unsafe end