From cc982e92d0329f27ea38e525b01f9adfbff26614 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Mon, 28 Oct 2013 11:19:25 -0400 Subject: [PATCH] security: Prevent arbitrary method invocation on conditions in if tag. --- History.md | 3 ++- lib/liquid/tags/if.rb | 4 +++- test/liquid/tags/if_else_tag_test.rb | 6 ++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/History.md b/History.md index 161688a..d402b5f 100644 --- a/History.md +++ b/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" diff --git a/lib/liquid/tags/if.rb b/lib/liquid/tags/if.rb index ce086ef..2225e8f 100644 --- a/lib/liquid/tags/if.rb +++ b/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 diff --git a/test/liquid/tags/if_else_tag_test.rb b/test/liquid/tags/if_else_tag_test.rb index 1282c2d..19869fb 100644 --- a/test/liquid/tags/if_else_tag_test.rb +++ b/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