diff --git a/gem_common.rb b/gem_common.rb index e1309baee3..38e0fff935 100644 --- a/gem_common.rb +++ b/gem_common.rb @@ -18,7 +18,7 @@ def self.extended_dependencies spec spec.add_dependency "terminal-table", "~>1.4" spec.add_dependency "highline", "~>2.0" spec.add_dependency "erubis", "~>2.6" - spec.add_dependency "haml", ">=3.0", "<5.0" + spec.add_dependency "haml", ">=5.1" spec.add_dependency "slim", ">=1.3.6", "<=4.0.1" end end diff --git a/lib/brakeman/checks/base_check.rb b/lib/brakeman/checks/base_check.rb index e5d2777b95..51366404a6 100644 --- a/lib/brakeman/checks/base_check.rb +++ b/lib/brakeman/checks/base_check.rb @@ -249,7 +249,12 @@ def get_location result raise ArgumentError end + begin location ||= (@current_template && @current_template.name) || @current_class || @current_module || @current_set || result[:location][:class] || result[:location][:template] || result[:location][:file].to_s + rescue => e + p result + raise e + end location = location[:name] if location.is_a? Hash location = location.name if location.is_a? Brakeman::Collection diff --git a/lib/brakeman/parsers/haml_embedded.rb b/lib/brakeman/parsers/haml_embedded.rb index 161c7137e8..a636ed558a 100644 --- a/lib/brakeman/parsers/haml_embedded.rb +++ b/lib/brakeman/parsers/haml_embedded.rb @@ -1,6 +1,6 @@ module Brakeman module FakeHamlFilter - # Copied from Haml - force delayed compilation + # Copied from Haml 4 - force delayed compilation def compile(compiler, text) filter = self compiler.instance_eval do diff --git a/lib/brakeman/parsers/template_parser.rb b/lib/brakeman/parsers/template_parser.rb index 58d14f1332..4c8a56d1cb 100644 --- a/lib/brakeman/parsers/template_parser.rb +++ b/lib/brakeman/parsers/template_parser.rb @@ -79,7 +79,9 @@ def parse_haml path, text Haml::Engine.new(text, :filename => path, - :escape_html => tracker.config.escape_html?).precompiled.gsub(/([^\\])\\n/, '\1') + :escape_html => tracker.config.escape_html?, + :escape_filter_interpolations => tracker.config.escape_filter_interpolations? + ).precompiled.gsub(/([^\\])\\n/, '\1') rescue Haml::Error => e tracker.error e, ["While compiling HAML in #{path}"] << e.backtrace nil diff --git a/lib/brakeman/processors/base_processor.rb b/lib/brakeman/processors/base_processor.rb index c6395f1ddc..8e654a1834 100644 --- a/lib/brakeman/processors/base_processor.rb +++ b/lib/brakeman/processors/base_processor.rb @@ -114,6 +114,8 @@ def process_block exp exp.unshift :rlist end + alias process_rlist process_block + #Processes the inside of an interpolated String. def process_evstr exp exp = exp.dup diff --git a/lib/brakeman/processors/haml_template_processor.rb b/lib/brakeman/processors/haml_template_processor.rb index 5009017d47..8114e592d4 100644 --- a/lib/brakeman/processors/haml_template_processor.rb +++ b/lib/brakeman/processors/haml_template_processor.rb @@ -2,8 +2,10 @@ #Processes HAML templates. class Brakeman::HamlTemplateProcessor < Brakeman::TemplateProcessor - HAML_FORMAT_METHOD = /format_script_(true|false)_(true|false)_(true|false)_(true|false)_(true|false)_(true|false)_(true|false)/ + HAMLOUT = s(:call, nil, :_hamlout) + HAML_BUFFER = s(:call, HAMLOUT, :buffer) HAML_HELPERS = s(:colon2, s(:const, :Haml), :Helpers) + HAML_HELPERS2 = s(:colon2, s(:colon3, :Haml), :Helpers) JAVASCRIPT_FILTER = s(:colon2, s(:colon2, s(:const, :Haml), :Filters), :Javascript) COFFEE_FILTER = s(:colon2, s(:colon2, s(:const, :Haml), :Filters), :Coffee) @@ -14,130 +16,52 @@ def initialize *args #Processes call, looking for template output def process_call exp - target = exp.target - if sexp? target - target = process target + exp = process_default exp + + if buffer_append? exp + output = normalize_output(exp.first_arg) + res = get_pushed_value(output) end - method = exp.method - - if (call? target and target.method == :_hamlout) - res = case method - when :adjust_tabs, :rstrip!, :attributes #Check attributes, maybe? - ignore - when :options, :buffer - exp - when :open_tag - process_call_args exp - else - arg = exp.first_arg - - if arg - @inside_concat = true - exp.first_arg = process(arg) - out = normalize_output(exp.first_arg) - @inside_concat = false - else - raise "Empty _hamlout.#{method}()?" - end - - if string? out - ignore - else - r = case method.to_s - when "push_text" - build_output_from_push_text(out) - when HAML_FORMAT_METHOD - if $4 == "true" - if string_interp? out - build_output_from_push_text(out, :escaped_output) - else - Sexp.new :format_escaped, out - end - else - if string_interp? out - build_output_from_push_text(out) - else - Sexp.new :format, out - end - end - - else - raise "Unrecognized action on _hamlout: #{method}" - end - - @javascript = false - r - end - end - - res.line(exp.line) - res - - #_hamlout.buffer << - #This seems to be used rarely, but directly appends args to output buffer. - #Has something to do with values of blocks? - elsif sexp? target and method == :<< and is_buffer_target? target - @inside_concat = true - exp.first_arg = process(exp.first_arg) - out = normalize_output(exp.first_arg) - @inside_concat = false - - if out.node_type == :str #ignore plain strings - ignore - else - add_output out - end - elsif target == nil and method == :render - #Process call to render() - exp.arglist = process exp.arglist - make_render_in_view exp - elsif target == nil and method == :find_and_preserve and exp.first_arg - process exp.first_arg - elsif method == :render_with_options - if target == JAVASCRIPT_FILTER or target == COFFEE_FILTER - @javascript = true - end + res or exp + end - process exp.first_arg - else - exp.target = target - exp.arglist = process exp.arglist - exp - end + # _haml_out.buffer << ... + def buffer_append? exp + call? exp and + exp.target == HAML_BUFFER and + exp.method == :<< + end + + def frozen_string_literal? exp + call? exp and + exp.method == :freeze and + string? exp.target + end + + PRESERVE_METHODS = [:find_and_preserve, :preserve] + + def find_and_preserve? exp + call? exp and + PRESERVE_METHODS.include?(exp.method) and + exp.first_arg end #If inside an output stream, only return the final expression def process_block exp exp = exp.dup exp.shift - if @inside_concat - @inside_concat = false - exp[0..-2].each do |e| - process e - end - @inside_concat = true - process exp[-1] - else - exp.map! do |e| - res = process e - if res.empty? - nil - else - res - end + + exp.map! do |e| + res = process e + if res.empty? + nil + else + res end - Sexp.new(:rlist).concat(exp).compact end - end - #Checks if the buffer is the target in a method call Sexp. - #TODO: Test this - def is_buffer_target? exp - exp.node_type == :call and - node_type? exp.target, :lvar and - exp.target.value == :_hamlout and - exp.method == :buffer + Sexp.new(:rlist).concat(exp).compact end #HAML likes to put interpolated values into _hamlout.push_text @@ -158,7 +82,6 @@ def build_output_from_push_text exp, default = :output end end - #Gets outputs from values interpolated into _hamlout.push_text def get_pushed_value exp, default = :output return exp unless sexp? exp @@ -173,24 +96,71 @@ def get_pushed_value exp, default = :output exp when :str, :ignore, :output, :escaped_output exp - when :block, :rlist, :dstr - exp.map! { |e| get_pushed_value e } + when :block, :rlist + exp.map! { |e| get_pushed_value(e, default) } + when :dstr + build_output_from_push_text(exp, default) when :if - clauses = [get_pushed_value(exp.then_clause), get_pushed_value(exp.else_clause)].compact + clauses = [get_pushed_value(exp.then_clause, default), get_pushed_value(exp.else_clause, default)].compact if clauses.length > 1 s(:or, *clauses).line(exp.line) else clauses.first end - else - if call? exp and exp.target == HAML_HELPERS and exp.method == :html_escape - add_escaped_output exp.first_arg - elsif @javascript and call? exp and (exp.method == :j or exp.method == :escape_javascript) - add_escaped_output exp.first_arg + when :call + if exp.method == :to_s or exp.method == :strip + get_pushed_value(exp.target, default) + elsif haml_helpers? exp.target and exp.method == :html_escape + get_pushed_value(exp.first_arg, :escaped_output) + elsif @javascript and (exp.method == :j or exp.method == :escape_javascript) # TODO: Remove - this is not safe + get_pushed_value(exp.first_arg, :escaped_output) + elsif find_and_preserve? exp or fix_textareas? exp + get_pushed_value(exp.first_arg, default) + elsif raw? exp + get_pushed_value(exp.first_arg, :output) + elsif hamlout_attributes? exp + ignore # ignore _hamlout.attributes calls + elsif exp.target.nil? and exp.method == :render + #Process call to render() + exp.arglist = process exp.arglist + make_render_in_view exp + elsif exp.method == :render_with_options + if exp.target == JAVASCRIPT_FILTER or exp.target == COFFEE_FILTER + @javascript = true + end + + get_pushed_value(exp.first_arg, default) + @javascript = false else add_output exp, default end + else + add_output exp, default end end + + def haml_helpers? exp + # Sometimes its Haml::Helpers and + # sometimes its ::Haml::Helpers + exp == HAML_HELPERS or + exp == HAML_HELPERS2 + end + + def hamlout_attributes? exp + call? exp and + exp.target == HAMLOUT and + exp.method == :attributes + end + + def fix_textareas? exp + call? exp and + exp.target == HAMLOUT and + exp.method == :fix_textareas! + end + + def raw? exp + call? exp and + exp.method == :raw + end end diff --git a/lib/brakeman/processors/template_alias_processor.rb b/lib/brakeman/processors/template_alias_processor.rb index edb6b91699..79b222d51d 100644 --- a/lib/brakeman/processors/template_alias_processor.rb +++ b/lib/brakeman/processors/template_alias_processor.rb @@ -32,6 +32,34 @@ def process_template name, args, _, line = nil end end + def process_lasgn exp + if exp.lhs == :haml_temp or haml_capture? exp.rhs + exp.rhs = process exp.rhs + + # Avoid propagating contents of block + if node_type? exp.rhs, :iter + new_exp = exp.dup + new_exp.rhs = exp.rhs.block_call + + super new_exp + + exp # Still save the original, though + else + super exp + end + else + super exp + end + end + + HAML_CAPTURE = [:capture, :capture_haml] + + def haml_capture? exp + node_type? exp, :iter and + call? exp.block_call and + HAML_CAPTURE.include? exp.block_call.method + end + #Determine template name def template_name name if !name.to_s.include?('/') && @template.name.to_s.include?('/') diff --git a/lib/brakeman/tracker/config.rb b/lib/brakeman/tracker/config.rb index 175da2860a..28292abae0 100644 --- a/lib/brakeman/tracker/config.rb +++ b/lib/brakeman/tracker/config.rb @@ -44,6 +44,12 @@ def escape_html_entities_in_json? true? @rails.dig(:active_support, :escape_html_entities_in_json) end + def escape_filter_interpolations? + # TODO see if app is actually turning this off itself + has_gem?(:haml) and + version_between? "5.0.0", "5.99", gem_version(:haml) + end + def whitelist_attributes? @rails.dig(:active_record, :whitelist_attributes) == Sexp.new(:true) end diff --git a/test/apps/rails4/app/views/users/haml_test.html.haml b/test/apps/rails4/app/views/users/haml_test.html.haml index 564e06f756..0ad47b5600 100644 --- a/test/apps/rails4/app/views/users/haml_test.html.haml +++ b/test/apps/rails4/app/views/users/haml_test.html.haml @@ -5,6 +5,6 @@ %h1= raw params[:y] =" #{User.first.name.html_safe}" :javascript - var import_file_upload_id = "#{j(params[:id])}"; + var import_file_upload_id = "#{j(params[:id1])}"; :coffeescript - import_file_upload_id_coffee = "#{j(params[:id])}" + import_file_upload_id_coffee = "#{j(params[:id2])}" diff --git a/test/apps/rails5/app/controllers/widget_controller.rb b/test/apps/rails5/app/controllers/widget_controller.rb index 2156571e9a..224d60a287 100644 --- a/test/apps/rails5/app/controllers/widget_controller.rb +++ b/test/apps/rails5/app/controllers/widget_controller.rb @@ -107,6 +107,9 @@ def render_safely slug = params[:slug].to_s render slug if template_exists?(slug, 'pages') end + + def attributes + end end IDENTIFIER_NAMESPACE = 'apis' diff --git a/test/apps/rails5/app/views/widget/attributes.html.haml b/test/apps/rails5/app/views/widget/attributes.html.haml new file mode 100644 index 0000000000..f4a4e48472 --- /dev/null +++ b/test/apps/rails5/app/views/widget/attributes.html.haml @@ -0,0 +1,23 @@ +%a{"data-text" => "#{params[:name]}"} No warning + +%pre.codeBlock.u-margin= params[:blah] + += @taxon = User.first + + += form_tag taxa_create_obsolete_combination_path do + %p + This page is for creating missing obsolete combination records. A new record will be created when "Create" is pressed. The new record will have: + %ul + %li its genus set to the obsolete genus + %li the name will be: obsolete genus name + #{@taxon.name.epithet_html.html_safe} + %li its current valid name set to #{@taxon.name_with_fossil} + %li + the status set to + %code obsolete combination + %li and the protonym set to the protonym of #{@taxon.name_with_fossil} + += blah = capture do + %p= params[:x] + += link_to stuff, blah diff --git a/test/tests/rails5.rb b/test/tests/rails5.rb index 3c999fc67d..7531378e2f 100644 --- a/test/tests/rails5.rb +++ b/test/tests/rails5.rb @@ -846,4 +846,27 @@ def test_reverse_tabnabbing :confidence => 1, :relative_path => "app/views/users/show.html.erb" end + + def test_haml_attributes + assert_no_warning :type => :template, + :warning_code => 2, + :warning_type => "Cross-Site Scripting", + :line => 1, + :message => /^Unescaped\ parameter\ value/, + :confidence => 2, + :relative_path => "app/views/widget/attributes.html.haml", + :code => s(:call, s(:call, nil, :_hamlout), :attributes, s(:hash, s(:str, "data-text"), s(:dstr, "", s(:evstr, s(:call, s(:params), :[], s(:lit, :name))))), s(:nil)), + :user_input => s(:call, s(:params), :[], s(:lit, :name)) + end + + def test_haml_textareas + assert_no_warning :type => :template, + :warning_code => 2, + :warning_type => "Cross-Site Scripting", + :line => 3, + :message => /^Unescaped\ parameter\ value/, + :confidence => 2, + :relative_path => "app/views/widget/attributes.html.haml", + :code => s(:call, s(:call, nil, :_hamlout), :fix_textareas!, s(:call, s(:colon2, s(:colon3, :Haml), :Helpers), :preserve, s(:call, s(:call, s(:colon2, s(:colon3, :Haml), :Helpers), :html_escape, s(:call, s(:call, s(:params), :[], s(:lit, :blah)), :to_s)), :strip))) + end end