Update Haml support from 4.x to 5.x

Fixes #1044

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

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

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

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

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

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

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?('/')

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

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

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'