Merge pull request #440 from Shopify/drop-tainting

Variable tainting

lib/liquid/errors.rb

@@ -54,5 +54,6 @@ module Liquid
   class StandardError < Error; end
   class SyntaxError < Error; end
   class StackLevelError < Error; end
+  class TaintedError < Error; end
   class MemoryError < Error; end
 end

lib/liquid/standardfilters.rb

@@ -34,7 +34,7 @@ module Liquid
     end
 
     def escape(input)
-      CGI.escapeHTML(input) rescue input
+      CGI.escapeHTML(input).untaint rescue input
     end
     alias_method :h, :escape
 

lib/liquid/template.rb

@@ -60,6 +60,12 @@ module Liquid
       # :strict will enforce correct syntax.
       attr_writer :error_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
+      attr_writer :taint_mode
+
       def file_system
         @@file_system
       end
@@ -80,6 +86,10 @@ module Liquid
         @error_mode || :lax
       end
 
+      def taint_mode
+        @taint_mode || :lax
+      end
+
       # Pass a module with filter methods which should be available
       # to all liquid views. Good for registering the standard library
       def register_filter(mod)

lib/liquid/variable.rb

@@ -94,6 +94,16 @@ module Liquid
         end
         filterargs << keyword_args unless keyword_args.empty?
         output = context.invoke(filter[0], output, *filterargs)
+      end.tap do |obj|
+        if obj.tainted?
+          case Template.taint_mode
+          when :warn
+            @warnings ||= []
+            @warnings << "variable '#{@name}' is tainted and was not escaped"
+          when :error
+            raise TaintedError, "Error - variable '#{@name}' is tainted and was not escaped"
+          end
+        end
       end
     end
   end

test/integration/drop_test.rb

@@ -48,6 +48,10 @@ class ProductDrop < Liquid::Drop
     ContextDrop.new
   end
 
+  def user_input
+    "foo".taint
+  end
+
   protected
     def callmenot
       "protected"
@@ -108,6 +112,30 @@ class DropsTest < Minitest::Test
     assert_equal '  ', tpl.render!('product' => ProductDrop.new)
   end
 
+  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 }}')
+      tpl.render!('product' => ProductDrop.new)
+      assert_match /tainted/, tpl.warnings.first
+    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
+
   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)

test/test_helper.rb

@@ -57,6 +57,14 @@ module Minitest
       Liquid::Strainer.class_variable_set(:@@filters, original_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