Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for taint checking #1268

Merged
merged 2 commits into from
Jul 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion lib/liquid/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/liquid/standardfilters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 0 additions & 17 deletions lib/liquid/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 1 addition & 22 deletions lib/liquid/variable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion liquid.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
34 changes: 1 addition & 33 deletions test/integration/drop_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ def context
ContextDrop.new
end

def user_input
(+"foo").taint
end

protected

def callmenot
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down
28 changes: 0 additions & 28 deletions test/integration/tags/render_tag_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}")
Expand Down
8 changes: 0 additions & 8 deletions test/integration/template_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 0 additions & 12 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down