From 065ccbc4aa5b4955dae9743509907bd9ad0db0b9 Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Thu, 23 Jul 2020 16:22:46 -0400 Subject: [PATCH 1/2] Remove support for taint checking --- lib/liquid/errors.rb | 1 - lib/liquid/standardfilters.rb | 2 +- lib/liquid/template.rb | 17 ------------ lib/liquid/variable.rb | 23 +--------------- test/integration/drop_test.rb | 34 +----------------------- test/integration/tags/render_tag_test.rb | 28 ------------------- test/integration/template_test.rb | 8 ------ test/test_helper.rb | 12 --------- 8 files changed, 3 insertions(+), 122 deletions(-) diff --git a/lib/liquid/errors.rb b/lib/liquid/errors.rb index 4937da380..ee22368d7 100644 --- a/lib/liquid/errors.rb +++ b/lib/liquid/errors.rb @@ -46,7 +46,6 @@ def message_prefix StandardError = Class.new(Error) SyntaxError = Class.new(Error) StackLevelError = Class.new(Error) - TaintedError = Class.new(Error) MemoryError = Class.new(Error) ZeroDivisionError = Class.new(Error) FloatDomainError = Class.new(Error) diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index f746ef6f9..0f4c9145c 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -41,7 +41,7 @@ def capitalize(input) end def escape(input) - CGI.escapeHTML(input.to_s).untaint unless input.nil? + CGI.escapeHTML(input.to_s) unless input.nil? end alias_method :h, :escape diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index 58ca6e969..189fcc046 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -64,23 +64,6 @@ class << self attr_accessor :error_mode Template.error_mode = :lax - attr_reader :taint_mode - - # Sets how strict the taint checker should be. - # :lax is the default, and ignores the taint flag completely - # :warn adds a warning, but does not interrupt the rendering - # :error raises an error when tainted output is used - # @deprecated Since it is being deprecated in ruby itself. - def taint_mode=(mode) - taint_supported = Object.new.taint.tainted? - if mode != :lax && !taint_supported - raise NotImplementedError, "#{RUBY_ENGINE} #{RUBY_VERSION} doesn't support taint checking" - end - @taint_mode = mode - end - - Template.taint_mode = :lax - attr_accessor :default_exception_renderer Template.default_exception_renderer = lambda do |exception| exception diff --git a/lib/liquid/variable.rb b/lib/liquid/variable.rb index 9c80bc3a7..addb495f7 100644 --- a/lib/liquid/variable.rb +++ b/lib/liquid/variable.rb @@ -86,9 +86,7 @@ def render(context) context.invoke(filter_name, output, *filter_args) end - obj = context.apply_global_filter(obj) - taint_check(context, obj) - obj + context.apply_global_filter(obj) end def render_to_output_buffer(context, output) @@ -142,25 +140,6 @@ def evaluate_filter_expressions(context, filter_args, filter_kwargs) parsed_args end - def taint_check(context, obj) - return if Template.taint_mode == :lax - return unless obj.tainted? - - @markup =~ QuotedFragment - name = Regexp.last_match(0) - - error = TaintedError.new("variable '#{name}' is tainted and was not escaped") - error.line_number = line_number - error.template_name = context.template_name - - case Template.taint_mode - when :warn - context.warnings << error - when :error - raise error - end - end - class ParseTreeVisitor < Liquid::ParseTreeVisitor def children [@node.name] + @node.filters.flatten diff --git a/test/integration/drop_test.rb b/test/integration/drop_test.rb index 81c822229..3ca14b654 100644 --- a/test/integration/drop_test.rb +++ b/test/integration/drop_test.rb @@ -49,10 +49,6 @@ def context ContextDrop.new end - def user_input - (+"foo").taint - end - protected def callmenot @@ -114,34 +110,6 @@ def test_product_drop assert_equal(' ', tpl.render!('product' => ProductDrop.new)) end - if taint_supported? - def test_rendering_raises_on_tainted_attr - with_taint_mode(:error) do - tpl = Liquid::Template.parse('{{ product.user_input }}') - assert_raises TaintedError do - tpl.render!('product' => ProductDrop.new) - end - end - end - - def test_rendering_warns_on_tainted_attr - with_taint_mode(:warn) do - tpl = Liquid::Template.parse('{{ product.user_input }}') - context = Context.new('product' => ProductDrop.new) - tpl.render!(context) - assert_equal [Liquid::TaintedError], context.warnings.map(&:class) - assert_equal "variable 'product.user_input' is tainted and was not escaped", context.warnings.first.to_s(false) - end - end - - def test_rendering_doesnt_raise_on_escaped_tainted_attr - with_taint_mode(:error) do - tpl = Liquid::Template.parse('{{ product.user_input | escape }}') - tpl.render!('product' => ProductDrop.new) - end - end - end - def test_drop_does_only_respond_to_whitelisted_methods assert_equal("", Liquid::Template.parse("{{ product.inspect }}").render!('product' => ProductDrop.new)) assert_equal("", Liquid::Template.parse("{{ product.pretty_inspect }}").render!('product' => ProductDrop.new)) @@ -281,7 +249,7 @@ def test_default_to_s_on_drops end def test_invokable_methods - assert_equal(%w(to_liquid catchall user_input context texts).to_set, ProductDrop.invokable_methods) + assert_equal(%w(to_liquid catchall context texts).to_set, ProductDrop.invokable_methods) assert_equal(%w(to_liquid scopes_as_array loop_pos scopes).to_set, ContextDrop.invokable_methods) assert_equal(%w(to_liquid size max min first count).to_set, EnumerableDrop.invokable_methods) assert_equal(%w(to_liquid max min sort count first).to_set, RealEnumerableDrop.invokable_methods) diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb index ae9390eba..b34b2cde0 100644 --- a/test/integration/tags/render_tag_test.rb +++ b/test/integration/tags/render_tag_test.rb @@ -42,34 +42,6 @@ def test_render_does_not_inherit_variable_with_same_name_as_snippet assert_template_result('', "{% assign snippet = 'should not be visible' %}{% render 'snippet' %}") end - if taint_supported? - def test_render_sets_the_correct_template_name_for_errors - Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}') - - with_taint_mode :error do - template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}') - context = Context.new('unsafe' => (+'unsafe').tap(&:taint)) - template.render(context) - - assert_equal [Liquid::TaintedError], template.errors.map(&:class) - assert_equal 'snippet', template.errors.first.template_name - end - end - - def test_render_sets_the_correct_template_name_for_warnings - Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}') - - with_taint_mode :warn do - template = Liquid::Template.parse('{% render "snippet", unsafe: unsafe %}') - context = Context.new('unsafe' => (+'unsafe').tap(&:taint)) - template.render(context) - - assert_equal [Liquid::TaintedError], context.warnings.map(&:class) - assert_equal 'snippet', context.warnings.first.template_name - end - end - end - def test_render_does_not_mutate_parent_scope Liquid::Template.file_system = StubFileSystem.new('snippet' => '{% assign inner = 1 %}') assert_template_result('', "{% render 'snippet' %}{{ inner }}") diff --git a/test/integration/template_test.rb b/test/integration/template_test.rb index b637f31a6..57d6e4fe0 100644 --- a/test/integration/template_test.rb +++ b/test/integration/template_test.rb @@ -405,12 +405,4 @@ def test_render_does_not_override_context_static_register_when_register_key_exis assert_nil(context.registers[:random_register]) assert(context.registers.key?(:random_register)) end - - unless taint_supported? - def test_taint_mode - assert_raises(NotImplementedError) do - Template.taint_mode = :warn - end - end - end end diff --git a/test/test_helper.rb b/test/test_helper.rb index fd08f8ee1..f3d5d2404 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -32,10 +32,6 @@ class Test def fixture(name) File.join(File.expand_path(__dir__), "fixtures", name) end - - def self.taint_supported? - Object.new.taint.tainted? - end end module Assertions @@ -93,14 +89,6 @@ def with_global_filter(*globals) Liquid::StrainerFactory.instance_variable_set(:@global_filters, original_global_filters) end - def with_taint_mode(mode) - old_mode = Liquid::Template.taint_mode - Liquid::Template.taint_mode = mode - yield - ensure - Liquid::Template.taint_mode = old_mode - end - def with_error_mode(mode) old_mode = Liquid::Template.error_mode Liquid::Template.error_mode = mode From 4970167726130557a5c0dc05932515b21c7e4caa Mon Sep 17 00:00:00 2001 From: Dylan Thacker-Smith Date: Thu, 23 Jul 2020 16:23:18 -0400 Subject: [PATCH 2/2] Bump rake development dependency Gets rid of a deprecation warning when running the tests. --- liquid.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/liquid.gemspec b/liquid.gemspec index 54a11fb95..cf1b75fc4 100644 --- a/liquid.gemspec +++ b/liquid.gemspec @@ -27,6 +27,6 @@ Gem::Specification.new do |s| s.require_path = "lib" - s.add_development_dependency('rake', '~> 11.3') + s.add_development_dependency('rake', '~> 13.0') s.add_development_dependency('minitest') end