From 2b40850e4a9c1fe807982d1f9abda794880f6384 Mon Sep 17 00:00:00 2001 From: Michael Go Date: Wed, 1 Feb 2023 19:52:30 -0400 Subject: [PATCH 1/2] fix variable lookup parse timing out with missing closing bracket --- lib/liquid.rb | 2 +- test/integration/variable_test.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/liquid.rb b/lib/liquid.rb index 09b0dd808..eba84e140 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -41,7 +41,7 @@ module Liquid AnyStartingTag = /#{TagStart}|#{VariableStart}/o PartialTemplateParser = /#{TagStart}.*?#{TagEnd}|#{VariableStart}.*?#{VariableIncompleteEnd}/om TemplateParser = /(#{PartialTemplateParser}|#{AnyStartingTag})/om - VariableParser = /\[(?:[^\[\]]+|\g<0>)*\]|#{VariableSegment}+\??/o + VariableParser = /\[(?>[^\[\]]+|\g<0>)*\]|#{VariableSegment}+\??/o RAISE_EXCEPTION_LAMBDA = ->(_e) { raise } diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index 0cb8bddc1..0667adb9a 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'test_helper' +require 'timeout' class VariableTest < Minitest::Test include Liquid @@ -169,4 +170,30 @@ def test_double_nested_variable_lookup } ) end + + def test_variable_lookup_should_not_hang_with_invalid_syntax + Timeout.timeout(1) do + assert_template_result( + 'bar', + "{{['foo'}}", + { + 'foo' => 'bar', + }, + error_mode: :lax, + ) + end + + very_long_key = "1234567890" * 100 + + Timeout.timeout(1) do + assert_template_result( + 'bar', + "{{['#{very_long_key}'}}", + { + very_long_key => 'bar', + }, + error_mode: :lax, + ) + end + end end From bd9c3802c88473a227afeb3e2ff8a0ad6518e5a0 Mon Sep 17 00:00:00 2001 From: Michael Go Date: Wed, 1 Feb 2023 21:15:35 -0400 Subject: [PATCH 2/2] add variable parser timeout unit tests --- test/integration/variable_test.rb | 26 +++++++++++++++++--------- test/unit/regexp_unit_test.rb | 13 +++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/test/integration/variable_test.rb b/test/integration/variable_test.rb index 0667adb9a..a66f1a0bd 100644 --- a/test/integration/variable_test.rb +++ b/test/integration/variable_test.rb @@ -185,15 +185,23 @@ def test_variable_lookup_should_not_hang_with_invalid_syntax very_long_key = "1234567890" * 100 - Timeout.timeout(1) do - assert_template_result( - 'bar', - "{{['#{very_long_key}'}}", - { - very_long_key => 'bar', - }, - error_mode: :lax, - ) + template_list = [ + "{{['#{very_long_key}']}}", # valid + "{{['#{very_long_key}'}}", # missing closing bracket + "{{[['#{very_long_key}']}}", # extra open bracket + ] + + template_list.each do |template| + Timeout.timeout(1) do + assert_template_result( + 'bar', + template, + { + very_long_key => 'bar', + }, + error_mode: :lax, + ) + end end end end diff --git a/test/unit/regexp_unit_test.rb b/test/unit/regexp_unit_test.rb index aad663c1b..877c95e45 100644 --- a/test/unit/regexp_unit_test.rb +++ b/test/unit/regexp_unit_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'test_helper' +require 'timeout' class RegexpUnitTest < Minitest::Test include Liquid @@ -37,10 +38,22 @@ def test_quoted_words_in_the_middle def test_variable_parser assert_equal(['var'], 'var'.scan(VariableParser)) + assert_equal(['[var]'], '[var]'.scan(VariableParser)) assert_equal(['var', 'method'], 'var.method'.scan(VariableParser)) assert_equal(['var', '[method]'], 'var[method]'.scan(VariableParser)) assert_equal(['var', '[method]', '[0]'], 'var[method][0]'.scan(VariableParser)) assert_equal(['var', '["method"]', '[0]'], 'var["method"][0]'.scan(VariableParser)) assert_equal(['var', '[method]', '[0]', 'method'], 'var[method][0].method'.scan(VariableParser)) end + + def test_variable_parser_with_large_input + Timeout.timeout(1) { assert_equal(['[var]'], '[var]'.scan(VariableParser)) } + + very_long_string = "foo" * 1000 + + # valid dynamic lookup + Timeout.timeout(1) { assert_equal(["[#{very_long_string}]"], "[#{very_long_string}]".scan(VariableParser)) } + # invalid dynamic lookup with missing closing bracket + Timeout.timeout(1) { assert_equal([very_long_string], "[#{very_long_string}".scan(VariableParser)) } + end end # RegexpTest