diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 4500aef..129b71a 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -39,6 +39,7 @@ module Liquid filters.each do |f| raise ArgumentError, "Expected module but got: #{f.class}" unless f.is_a?(Module) + Strainer.add_known_filter(f) strainer.extend(f) end end @@ -71,11 +72,7 @@ module Liquid end def invoke(method, *args) - if strainer.respond_to?(method) - strainer.__send__(method, *args) - else - args.first - end + strainer.invoke(method, *args) end # Push new local scope on the stack. use Context#stack instead diff --git a/lib/liquid/drop.rb b/lib/liquid/drop.rb index ee2a4d7..88adb6b 100644 --- a/lib/liquid/drop.rb +++ b/lib/liquid/drop.rb @@ -1,3 +1,5 @@ +require 'set' + module Liquid # A drop in liquid is a class which allows you to export DOM like things to liquid. @@ -31,8 +33,8 @@ module Liquid # called by liquid to invoke a drop def invoke_drop(method_or_key) - if method_or_key && method_or_key != EMPTY_STRING && self.class.public_method_defined?(method_or_key.to_s.to_sym) - send(method_or_key.to_s.to_sym) + if method_or_key && method_or_key != EMPTY_STRING && self.class.invokable?(method_or_key) + send(method_or_key) else before_method(method_or_key) end @@ -47,5 +49,13 @@ module Liquid end alias :[] :invoke_drop + + private + + # Check for method existence without invoking respond_to?, which creates symbols + def self.invokable?(method_name) + @invokable_methods ||= Set.new((public_instance_methods - Liquid::Drop.public_instance_methods).map(&:to_s)) + @invokable_methods.include?(method_name.to_s) + end end end diff --git a/lib/liquid/strainer.rb b/lib/liquid/strainer.rb index 445a0ae..15aefa4 100644 --- a/lib/liquid/strainer.rb +++ b/lib/liquid/strainer.rb @@ -2,24 +2,15 @@ require 'set' module Liquid - parent_object = if defined? BlankObject - BlankObject - else - Object - end - # Strainer is the parent class for the filters system. - # New filters are mixed into the strainer class which is then instanciated for each liquid template render run. + # New filters are mixed into the strainer class which is then instantiated for each liquid template render run. # - # One of the strainer's responsibilities is to keep malicious method calls out - class Strainer < parent_object #:nodoc: - INTERNAL_METHOD = /^__/ - @@required_methods = Set.new([:__id__, :__send__, :respond_to?, :kind_of?, :extend, :methods, :singleton_methods, :class, :object_id]) - - # Ruby 1.9.2 introduces Object#respond_to_missing?, which is invoked by Object#respond_to? - @@required_methods << :respond_to_missing? if Object.respond_to? :respond_to_missing? - + # The Strainer only allows method calls defined in filters given to it via Strainer.global_filter, + # Context#add_filters or Template.register_filter + class Strainer #:nodoc: @@filters = {} + @@known_filters = Set.new + @@known_methods = Set.new def initialize(context) @context = context @@ -27,28 +18,36 @@ module Liquid def self.global_filter(filter) raise ArgumentError, "Passed filter is not a module" unless filter.is_a?(Module) + add_known_filter(filter) @@filters[filter.name] = filter end + def self.add_known_filter(filter) + unless @@known_filters.include?(filter) + @@method_blacklist ||= Set.new(Strainer.instance_methods.map(&:to_s)) + new_methods = filter.instance_methods.map(&:to_s) + new_methods.reject!{ |m| @@method_blacklist.include?(m) } + @@known_methods.merge(new_methods) + @@known_filters.add(filter) + end + end + def self.create(context) strainer = Strainer.new(context) @@filters.each { |k,m| strainer.extend(m) } strainer end - def respond_to?(method, include_private = false) - method_name = method.to_s - return false if method_name =~ INTERNAL_METHOD - return false if @@required_methods.include?(method_name) - super + def invoke(method, *args) + if invokable?(method) + send(method, *args) + else + args.first + end end - # remove all standard methods from the bucket so circumvent security - # problems - instance_methods.each do |m| - unless @@required_methods.include?(m.to_sym) - undef_method m - end + def invokable?(method) + @@known_methods.include?(method.to_s) && respond_to?(method) end end end diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 2c317a8..5758fb0 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -26,7 +26,7 @@ module Liquid if matches = f.match(/\s*(\w+)(?:\s*#{FilterArgumentSeparator}(.*))?/) filtername = matches[1] filterargs = matches[2].to_s.scan(/(?:\A|#{ArgumentSeparator})\s*((?:\w+\s*\:\s*)?#{QuotedFragment})/o).flatten - @filters << [filtername.to_sym, filterargs] + @filters << [filtername, filterargs] end end end diff --git a/test/liquid/context_test.rb b/test/liquid/context_test.rb index c2c316f..e50b237 100644 --- a/test/liquid/context_test.rb +++ b/test/liquid/context_test.rb @@ -189,10 +189,10 @@ class ContextTest < Test::Unit::TestCase end context = Context.new - methods_before = context.strainer.methods.map { |method| method.to_s } + assert_equal "Wookie", context.invoke("hi", "Wookie") + context.add_filters(filter) - methods_after = context.strainer.methods.map { |method| method.to_s } - assert_equal (methods_before + ["hi"]).sort, methods_after.sort + assert_equal "Wookie hi!", context.invoke("hi", "Wookie") end def test_add_item_in_outer_scope diff --git a/test/liquid/drop_test.rb b/test/liquid/drop_test.rb index ebac1e5..7819f64 100644 --- a/test/liquid/drop_test.rb +++ b/test/liquid/drop_test.rb @@ -115,6 +115,13 @@ class DropsTest < Test::Unit::TestCase assert_equal ' ', output end + def test_object_methods_not_allowed + [:dup, :clone, :singleton_class, :eval, :class_eval, :inspect].each do |method| + output = Liquid::Template.parse(" {{ product.#{method} }} ").render('product' => ProductDrop.new) + assert_equal ' ', output + end + end + def test_scope assert_equal '1', Liquid::Template.parse( '{{ context.scopes }}' ).render('context' => ContextDrop.new) assert_equal '2', Liquid::Template.parse( '{%for i in dummy%}{{ context.scopes }}{%endfor%}' ).render('context' => ContextDrop.new, 'dummy' => [1]) diff --git a/test/liquid/security_test.rb b/test/liquid/security_test.rb index 413cdf6..0e9ac33 100644 --- a/test/liquid/security_test.rb +++ b/test/liquid/security_test.rb @@ -38,4 +38,27 @@ class SecurityTest < Test::Unit::TestCase assert_equal expected, Template.parse(text).render(@assigns, :filters => SecurityFilter) end + + def test_does_not_add_filters_to_symbol_table + current_symbols = Symbol.all_symbols + + test = %( {{ "some_string" | a_bad_filter }} ) + + template = Template.parse(test) + assert_equal [], (Symbol.all_symbols - current_symbols) + + template.render + assert_equal [], (Symbol.all_symbols - current_symbols) + end + + def test_does_not_add_drop_methods_to_symbol_table + current_symbols = Symbol.all_symbols + + drop = Drop.new + drop.invoke_drop("custom_method_1") + drop.invoke_drop("custom_method_2") + drop.invoke_drop("custom_method_3") + + assert_equal [], (Symbol.all_symbols - current_symbols) + end end # SecurityTest diff --git a/test/liquid/strainer_test.rb b/test/liquid/strainer_test.rb index fc92e10..582ed7f 100644 --- a/test/liquid/strainer_test.rb +++ b/test/liquid/strainer_test.rb @@ -3,23 +3,50 @@ require 'test_helper' class StrainerTest < Test::Unit::TestCase include Liquid + module AccessScopeFilters + def public_filter + "public" + end + + def private_filter + "private" + end + private :private_filter + end + + Strainer.global_filter(AccessScopeFilters) + def test_strainer strainer = Strainer.create(nil) - assert_equal false, strainer.respond_to?('__test__') - assert_equal false, strainer.respond_to?('test') - assert_equal false, strainer.respond_to?('instance_eval') - assert_equal false, strainer.respond_to?('__send__') - assert_equal true, strainer.respond_to?('size') # from the standard lib + assert_equal 5, strainer.invoke('size', 'input') + assert_equal "public", strainer.invoke("public_filter") end - def test_should_respond_to_two_parameters + def test_strainer_only_invokes_public_filter_methods strainer = Strainer.create(nil) - assert_equal true, strainer.respond_to?('size', false) + assert_equal false, strainer.invokable?('__test__') + assert_equal false, strainer.invokable?('test') + assert_equal false, strainer.invokable?('instance_eval') + assert_equal false, strainer.invokable?('__send__') + assert_equal true, strainer.invokable?('size') # from the standard lib end - # Asserts that Object#respond_to_missing? is not being undefined in Ruby versions where it has been implemented - # Currently this method is only present in Ruby v1.9.2, or higher - def test_object_respond_to_missing - assert_equal Object.respond_to?(:respond_to_missing?), Strainer.create(nil).respond_to?(:respond_to_missing?) + def test_strainer_returns_nil_if_no_filter_method_found + strainer = Strainer.create(nil) + assert_nil strainer.invoke("private_filter") + assert_nil strainer.invoke("undef_the_filter") end + + def test_strainer_returns_first_argument_if_no_method_and_arguments_given + strainer = Strainer.create(nil) + assert_equal "password", strainer.invoke("undef_the_method", "password") + end + + def test_strainer_only_allows_methods_defined_in_filters + strainer = Strainer.create(nil) + assert_equal "1 + 1", strainer.invoke("instance_eval", "1 + 1") + assert_equal "puts", strainer.invoke("__send__", "puts", "Hi Mom") + assert_equal "has_method?", strainer.invoke("invoke", "has_method?", "invoke") + end + end # StrainerTest diff --git a/test/liquid/variable_test.rb b/test/liquid/variable_test.rb index 58a1ee3..217a885 100644 --- a/test/liquid/variable_test.rb +++ b/test/liquid/variable_test.rb @@ -11,71 +11,71 @@ class VariableTest < Test::Unit::TestCase def test_filters var = Variable.new('hello | textileze') assert_equal 'hello', var.name - assert_equal [[:textileze,[]]], var.filters + assert_equal [["textileze",[]]], var.filters var = Variable.new('hello | textileze | paragraph') assert_equal 'hello', var.name - assert_equal [[:textileze,[]], [:paragraph,[]]], var.filters + assert_equal [["textileze",[]], ["paragraph",[]]], var.filters var = Variable.new(%! hello | strftime: '%Y'!) assert_equal 'hello', var.name - assert_equal [[:strftime,["'%Y'"]]], var.filters + assert_equal [["strftime",["'%Y'"]]], var.filters var = Variable.new(%! 'typo' | link_to: 'Typo', true !) assert_equal %!'typo'!, var.name - assert_equal [[:link_to,["'Typo'", "true"]]], var.filters + assert_equal [["link_to",["'Typo'", "true"]]], var.filters var = Variable.new(%! 'typo' | link_to: 'Typo', false !) assert_equal %!'typo'!, var.name - assert_equal [[:link_to,["'Typo'", "false"]]], var.filters + assert_equal [["link_to",["'Typo'", "false"]]], var.filters var = Variable.new(%! 'foo' | repeat: 3 !) assert_equal %!'foo'!, var.name - assert_equal [[:repeat,["3"]]], var.filters + assert_equal [["repeat",["3"]]], var.filters var = Variable.new(%! 'foo' | repeat: 3, 3 !) assert_equal %!'foo'!, var.name - assert_equal [[:repeat,["3","3"]]], var.filters + assert_equal [["repeat",["3","3"]]], var.filters var = Variable.new(%! 'foo' | repeat: 3, 3, 3 !) assert_equal %!'foo'!, var.name - assert_equal [[:repeat,["3","3","3"]]], var.filters + assert_equal [["repeat",["3","3","3"]]], var.filters var = Variable.new(%! hello | strftime: '%Y, okay?'!) assert_equal 'hello', var.name - assert_equal [[:strftime,["'%Y, okay?'"]]], var.filters + assert_equal [["strftime",["'%Y, okay?'"]]], var.filters var = Variable.new(%! hello | things: "%Y, okay?", 'the other one'!) assert_equal 'hello', var.name - assert_equal [[:things,["\"%Y, okay?\"","'the other one'"]]], var.filters + assert_equal [["things",["\"%Y, okay?\"","'the other one'"]]], var.filters end def test_filter_with_date_parameter var = Variable.new(%! '2006-06-06' | date: "%m/%d/%Y"!) assert_equal "'2006-06-06'", var.name - assert_equal [[:date,["\"%m/%d/%Y\""]]], var.filters + assert_equal [["date",["\"%m/%d/%Y\""]]], var.filters end def test_filters_without_whitespace var = Variable.new('hello | textileze | paragraph') assert_equal 'hello', var.name - assert_equal [[:textileze,[]], [:paragraph,[]]], var.filters + assert_equal [["textileze",[]], ["paragraph",[]]], var.filters var = Variable.new('hello|textileze|paragraph') assert_equal 'hello', var.name - assert_equal [[:textileze,[]], [:paragraph,[]]], var.filters + assert_equal [["textileze",[]], ["paragraph",[]]], var.filters var = Variable.new("hello|replace:'foo','bar'|textileze") assert_equal 'hello', var.name - assert_equal [[:replace, ["'foo'", "'bar'"]], [:textileze, []]], var.filters + assert_equal [["replace", ["'foo'", "'bar'"]], ["textileze", []]], var.filters end def test_symbol var = Variable.new("http://disney.com/logo.gif | image: 'med' ") assert_equal 'http://disney.com/logo.gif', var.name - assert_equal [[:image,["'med'"]]], var.filters + assert_equal [["image",["'med'"]]], var.filters end def test_string_single_quoted @@ -111,7 +111,7 @@ class VariableTest < Test::Unit::TestCase def test_filter_with_keyword_arguments var = Variable.new(%! hello | things: greeting: "world", farewell: 'goodbye'!) assert_equal 'hello', var.name - assert_equal [[:things,["greeting: \"world\"","farewell: 'goodbye'"]]], var.filters + assert_equal [['things',["greeting: \"world\"","farewell: 'goodbye'"]]], var.filters end end