Merge pull request #1268 from Shopify/remove-taint-checking

Remove support for taint checking

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)

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
 

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

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

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

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)

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 }}")

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

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