Convert Strainer to white-list method protection

After moving the method existence check from Context into Strainer,
updated Strainer to only accept invokation methods that were added via
filter Modules, and done in a way that respond_to? is never called,
preventing unconstrained Symbol table growth.

lib/liquid/context.rb

@@ -71,11 +71,7 @@ module Liquid
     end
 
     def invoke(method, *args)
-      if strainer.respond_to?(method)
+      strainer.invoke(method, *args)
-        strainer.__send__(method, *args)
-      else
-        args.first
-      end
     end
 
     # Push new local scope on the stack. use <tt>Context#stack</tt> instead

lib/liquid/strainer.rb

@@ -9,16 +9,11 @@ module Liquid
   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
+  # 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 < 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?
-
     @@filters = {}
 
     def initialize(context)
@@ -36,19 +31,22 @@ module Liquid
       strainer
     end
 
-    def respond_to?(method, include_private = false)
+    def invoke(method, *args)
-      method_name = method.to_s
+      if has_method?(method)
-      return false if method_name =~ INTERNAL_METHOD
+        send(method, *args)
-      return false if @@required_methods.include?(method_name)
+      else
-      super
+        args.first
-    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
     end
+
+    private
+
+    def has_method?(method)
+      methods_to_check = self.methods - self.class.public_instance_methods
+      methods_to_check.any? do |instance_method|
+        instance_method.to_s == method.to_s
+      end
+    end
+
   end
 end

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 "Wookie hi!", context.invoke("hi", "Wookie")
-    assert_equal (methods_before + ["hi"]).sort, methods_after.sort
   end
 
   def test_add_item_in_outer_scope

test/liquid/strainer_test.rb

@@ -3,23 +3,57 @@ require 'test_helper'
 class StrainerTest < Test::Unit::TestCase
   include Liquid
 
-  def test_strainer
+  module AccessScopeFilters
-    strainer = Strainer.create(nil)
+    def public_filter
-    assert_equal false, strainer.respond_to?('__test__')
+      "public"
-    assert_equal false, strainer.respond_to?('test')
+    end
-    assert_equal false, strainer.respond_to?('instance_eval')
+
-    assert_equal false, strainer.respond_to?('__send__')
+    def private_filter
-    assert_equal true, strainer.respond_to?('size') # from the standard lib
+      "private"
+    end
+    private :private_filter
   end
 
-  def test_should_respond_to_two_parameters
+  module TestingFilter
-    strainer = Strainer.create(nil)
+    def test1
-    assert_equal true, strainer.respond_to?('size', false)
+      "test1"
+    end
+
+    def test2
+      "test2"
+    end
   end
 
-  # Asserts that Object#respond_to_missing? is not being undefined in Ruby versions where it has been implemented
+  Strainer.global_filter(AccessScopeFilters)
-  # Currently this method is only present in Ruby v1.9.2, or higher
+  Strainer.global_filter(TestingFilter)
-  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_only_invokes_public_filter_methods
+    strainer = Strainer.create(nil)
+    assert_equal "public", strainer.invoke("public_filter")
   end
+
+  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_allows_multiple_filters
+    strainer = Strainer.create(nil)
+    assert_equal "test1", strainer.invoke("test1")
+    assert_equal "test2", strainer.invoke("test2")
+  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