Merge pull request #274 from Shopify/restrict-send-on-conditions

security: Prevent arbitrary method invocation on conditions in if tag.

History.md

@@ -10,7 +10,8 @@
 * Fix clashing method names in enumerable drops, see #238 [Florian Weingarten, fw42]
 * Make map filter work on enumerable drops, see #233 [Florian Weingarten, fw42]
 * Improved whitespace stripping for blank blocks, related to #216 [Florian Weingarten, fw42]
-* Don't call to_sym when creating conditions and use public_send for security reasons, see #273 [Bouke van der Bijl, bouk]
+* Don't call to_sym when creating conditions for security reasons, see #273 [Bouke van der Bijl, bouk]
+* Prevent arbitrary method invocation on condition objects, see #274 [Dylan Thacker-Smith, dylanahsmith]
 
 ## 2.6.0 / not yet released / branch "2.6-stable"
 

lib/liquid/tags/if.rb

@@ -12,6 +12,7 @@ module Liquid
   class If < Block
     Syntax = /(#{QuotedFragment})\s*([=!<>a-z_]+)?\s*(#{QuotedFragment})?/o
     ExpressionsAndOperators = /(?:\b(?:\s?and\s?|\s?or\s?)\b|(?:\s*(?!\b(?:\s?and\s?|\s?or\s?)\b)(?:#{QuotedFragment}|\S+)\s*)+)/o
+    BOOLEAN_OPERATORS = %w(and or)
 
     def initialize(tag_name, markup, tokens)
       @blocks = []
@@ -63,7 +64,8 @@ module Liquid
           raise(SyntaxError.new(options[:locale].t("errors.syntax.if"))) unless expressions.shift.to_s =~ Syntax
 
           new_condition = Condition.new($1, $2, $3)
-          new_condition.public_send(operator, condition)
+          raise(SyntaxError.new(options[:locale].t("errors.syntax.if"))) unless BOOLEAN_OPERATORS.include?(operator)
+          new_condition.send(operator, condition)
           condition = new_condition
         end
 

test/liquid/tags/if_else_tag_test.rb

@@ -157,4 +157,10 @@ class IfElseTagTest < Test::Unit::TestCase
     assert_template_result('yes',
                            %({% if 'gnomeslab-and-or-liquid' contains 'gnomeslab-and-or-liquid' %}yes{% endif %}))
   end
+
+  def test_operators_are_whitelisted
+    assert_raise(SyntaxError) do
+      assert_template_result('', %({% if 1 or throw or or 1 %}yes{% endif %}))
+    end
+  end
 end # IfElseTest