Merge pull request #190 from presidentbeef/update_ruby_parser

Update to use RubyParser 3.x

brakeman.gemspec

@@ -12,8 +12,8 @@ Gem::Specification.new do |s|
   s.license = "MIT"
   s.add_dependency "activesupport"
   s.add_dependency "i18n"
-  s.add_dependency "ruby_parser", "2.3.1"
-  s.add_dependency "ruby2ruby", "~>1.2"
+  s.add_dependency "ruby_parser", "~>3.0.4"
+  s.add_dependency "ruby2ruby", "~>2.0"
   s.add_dependency "terminal-table", "~>1.4"
   s.add_dependency "fastercsv", "~>1.5"
   s.add_dependency "highline", "~>1.6"

lib/brakeman.rb

@@ -250,8 +250,6 @@ module Brakeman
     #Start scanning
     scanner = Scanner.new options
 
-    notify "[Notice] Using Ruby #{RUBY_VERSION}. Please make sure this matches the one used to run your Rails application."
-
     notify "Processing application in #{options[:app_path]}"
     tracker = scanner.process
 

lib/brakeman/checks/base_check.rb

@@ -72,7 +72,7 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
         @has_user_input = Match.new(:cookies, exp)
       elsif request_env? target
         @has_user_input = Match.new(:request, exp)
-      elsif sexp? target and model_name? target[1]
+      elsif sexp? target and model_name? target[1] #TODO: Can this be target.target?
         @has_user_input = Match.new(:model, exp)
       end
     end
@@ -166,8 +166,8 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
         matches = tracker.check_initializers([], :attr_accessible)
 
         matches.each do |result|
-          if result[1] == :ActiveRecord and result[2] == :Base
-            arg = result[-1][3][1]
+          if result[1] == "ActiveRecord" and result[2] == :Base
+            arg = result[-1].first_arg
 
             if arg.nil? or node_type? arg, :nil
               @mass_assign_disabled = true
@@ -177,9 +177,12 @@ class Brakeman::BaseCheck < Brakeman::SexpProcessor
         end
       else
         matches.each do |result|
-          if result[-1][3] == Sexp.new(:arglist, Sexp.new(:lit, :attr_accessible), Sexp.new(:nil))
-            @mass_assign_disabled = true
-            break
+          if call? result[-1]
+            call = result[-1]
+            if call.first_arg == Sexp.new(:lit, :attr_accessible) and call.second_arg == Sexp.new(:nil)
+              @mass_assign_disabled = true
+              break
+            end
           end
         end
       end

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -31,7 +31,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
 
   CGI = Sexp.new(:const, :CGI)
 
-  FORM_BUILDER = Sexp.new(:call, Sexp.new(:const, :FormBuilder), :new, Sexp.new(:arglist))
+  FORM_BUILDER = Sexp.new(:call, Sexp.new(:const, :FormBuilder), :new)
 
   #Run check
   def run_check
@@ -115,7 +115,11 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
         :confidence => CONFIDENCE[:high]
 
     elsif not tracker.options[:ignore_model_output] and match = has_immediate_model?(out)
-      method = match[2]
+      method = if call? match
+                 match.method
+               else
+                 nil
+               end
 
       unless IGNORE_MODEL_METHODS.include? method
         add_result out
@@ -241,7 +245,7 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
 
       #exp[0] = :ignore #should not be necessary
       @matched = false
-    elsif sexp? target and model_name? target[1]
+    elsif sexp? target and model_name? target[1] #TODO: use method call?
       @matched = Match.new(:model, exp)
     elsif cookies? exp
       @matched = Match.new(:cookies, exp)
@@ -286,9 +290,8 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
 
   #Ignore condition in if Sexp
   def process_if exp
-    exp[2..-1].each do |e|
-      process e if sexp? e
-    end
+    process exp.then_clause if sexp? exp.then_clause
+    process exp.else_clause if sexp? exp.else_clause
     exp
   end
 

lib/brakeman/checks/check_evaluation.rb

@@ -20,7 +20,7 @@ class Brakeman::CheckEvaluation < Brakeman::BaseCheck
 
   #Warns if eval includes user input
   def process_result result
-    if input = include_user_input?(result[:call][-1])
+    if input = include_user_input?(result[:call].arglist)
       warn :result => result,
         :warning_type => "Dangerous Eval",
         :message => "User input in eval",

lib/brakeman/checks/check_execute.rb

@@ -30,12 +30,12 @@ class Brakeman::CheckExecute < Brakeman::BaseCheck
   #Processes results from Tracker#find_call.
   def process_result result
     call = result[:call]
-
-    args = process call[3]
+    args = call.arglist
+    first_arg = call.first_arg
 
     case call.method
     when :system, :exec
-      failure = include_user_input?(args[1]) || include_interp?(args[1])
+      failure = include_user_input?(first_arg) || include_interp?(first_arg)
     else
       failure = include_user_input?(args) || include_interp?(args)
     end

lib/brakeman/checks/check_link_to.rb

@@ -82,7 +82,7 @@ class Brakeman::CheckLinkTo < Brakeman::CheckCrossSiteScripting
         :link_path => "link_to"
 
     elsif not tracker.options[:ignore_model_output] and match = has_immediate_model?(arg)
-      method = match[2]
+      method = match.method
 
       unless IGNORE_MODEL_METHODS.include? method
         add_result result

lib/brakeman/checks/check_mass_assignment.rb

@@ -78,7 +78,7 @@ class Brakeman::CheckMassAssignment < Brakeman::BaseCheck
 
   #Want to ignore calls to Model.new that have no arguments
   def check_call call
-    args = process_all call.args
+    args = process_all! call.args
 
     if args.empty? #empty new()
       false

lib/brakeman/checks/check_redirect.rb

@@ -73,12 +73,12 @@ class Brakeman::CheckRedirect < Brakeman::BaseCheck
     elsif call? arg
       if request_value? arg
         return Match.new(immediate, arg)
-      elsif request_value? arg[1]
-        return Match.new(immediate, arg[1])
-      elsif arg[2] == :url_for and include_user_input? arg
+      elsif request_value? arg.target
+        return Match.new(immediate, arg.target)
+      elsif arg.method == :url_for and include_user_input? arg
         return Match.new(immediate, arg)
         #Ignore helpers like some_model_url?
-      elsif arg[2].to_s =~ /_(url|path)\z/
+      elsif arg.method.to_s =~ /_(url|path)\z/
         return false
       end
     elsif request_value? arg

lib/brakeman/checks/check_send.rb

@@ -16,7 +16,7 @@ class Brakeman::CheckSend < Brakeman::BaseCheck
   end
 
   def process_result result
-    args = process_all result[:call].args
+    args = process_all! result[:call].args
     target = process result[:call].target
 
     if input = has_immediate_user_input?(args.first)

lib/brakeman/checks/check_session_settings.rb

@@ -10,7 +10,7 @@ class Brakeman::CheckSessionSettings < Brakeman::BaseCheck
     super
 
     if tracker.options[:rails3]
-      @session_settings = Sexp.new(:call, Sexp.new(:colon2, Sexp.new(:const, :Rails3), :Application), :config, Sexp.new(:arglist))
+      @session_settings = Sexp.new(:call, Sexp.new(:colon2, Sexp.new(:const, :Rails3), :Application), :config)
     else
       @session_settings = Sexp.new(:colon2, Sexp.new(:const, :ActionController), :Base)
     end

lib/brakeman/checks/check_single_quotes.rb

@@ -52,7 +52,7 @@ class Brakeman::CheckSingleQuotes < Brakeman::BaseCheck
   def process_class exp
     if exp.class_name == :ERB
       @inside_erb = true
-      process exp.body
+      process_all exp.body
       @inside_erb = false
     end
 
@@ -65,7 +65,7 @@ class Brakeman::CheckSingleQuotes < Brakeman::BaseCheck
   def process_module exp
     if @inside_erb and exp.module_name == :Util
       @inside_util = true
-      process exp.body
+      process_all exp.body
       @inside_util = false
     end
 
@@ -78,7 +78,7 @@ class Brakeman::CheckSingleQuotes < Brakeman::BaseCheck
   def process_defn exp
     if @inside_util and exp.method_name == :html_escape
       @inside_html_escape = true
-      process exp.body
+      process_all exp.body
       @inside_html_escape = false
     end
 

lib/brakeman/checks/check_sql.rb

@@ -61,7 +61,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
       active_record_models.each do |name, model|
         if model[:options][:named_scope]
           model[:options][:named_scope].each do |args|
-            call = Sexp.new(:call, nil, :named_scope, args).line(args.line)
+            call = make_call(nil, :named_scope, args).line(args.line)
             scope_calls << { :call => call, :location => [:class, name ], :method => :named_scope }
           end
         end
@@ -80,7 +80,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
               call = second_arg
               scope_calls << { :call => call, :location => [:class, name ], :method => call.method }
             else
-              call = Sexp.new(:call, nil, :scope, args).line(args.line)
+              call = make_call(nil, :scope, args).line(args.line)
               scope_calls << { :call => call, :location => [:class, name ], :method => :scope }
             end
           end
@@ -226,8 +226,8 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
         :confidence => confidence
     end
 
-    if check_for_limit_or_offset_vulnerability args[-1]
-      if include_user_input? args[-1]
+    if check_for_limit_or_offset_vulnerability args.last
+      if include_user_input? args.last
         confidence = CONFIDENCE[:high]
       else
         confidence = CONFIDENCE[:low]
@@ -261,23 +261,25 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
 
   def check_scope_arguments args
     return unless node_type? args, :arglist
+    scope_arg = args[2] #first arg is name of scope
 
-    if node_type? args[2], :iter
-      unsafe_sql? args[2][-1]
+    if node_type? scope_arg, :iter
+      unsafe_sql? scope_arg.block
     else
-      unsafe_sql? args[2]
+      unsafe_sql? scope_arg
     end
   end
 
   def check_query_arguments arg
     return unless sexp? arg
+    first_arg = arg[1]
 
     if node_type? arg, :arglist
-      if arg.length > 2 and node_type? arg[1], :string_interp, :dstr
+      if arg.length > 2 and node_type? first_arg, :string_interp, :dstr
         # Model.where("blah = ?", blah)
-        return check_string_interp arg[1]
+        return check_string_interp first_arg
       else
-        arg = arg[1]
+        arg = first_arg
       end
     end
 
@@ -319,7 +321,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
   def check_by_sql_arguments arg
     return unless sexp? arg
 
-    #This is kind of necessary, because unsafe_sql? will handle an array
+    #This is kind of unnecessary, because unsafe_sql? will handle an array
     #correctly, but might be better to be explicit.
     if array? arg
       unsafe_sql? arg[1]
@@ -457,7 +459,7 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
   def check_hash_values exp
     hash_iterate(exp) do |key, value|
       if symbol? key
-        unsafe = case key[1]
+        unsafe = case key.value
                  when :conditions, :having, :select
                    check_query_arguments value
                  when :order, :group

lib/brakeman/checks/check_validation_regex.rb

@@ -53,8 +53,9 @@ class Brakeman::CheckValidationRegex < Brakeman::BaseCheck
   #Get the name of the attribute being validated.
   def get_name validator
     name = validator[1]
+
     if sexp? name
-      name[1]
+      name.value
     else
       name
     end

lib/brakeman/processor.rb

@@ -54,7 +54,7 @@ module Brakeman
     #Process a model source
     def process_model src, file_name
       result = ModelProcessor.new(@tracker).process_model src, file_name
-      AliasProcessor.new(@tracker).process result
+      AliasProcessor.new(@tracker).process_all result if result
     end
 
     #Process either an ERB or HAML template

lib/brakeman/processors/alias_processor.rb

@@ -70,11 +70,9 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     @exp_context.push exp
 
     begin
-      exp.each_with_index do |e, i|
-        next if i == 0
-
+      exp.map! do |e|
         if sexp? e and not e.empty?
-          exp[i] = process e
+          process e
         else
           e
         end
@@ -107,7 +105,6 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
 
     target = exp.target
     method = exp.method
-    args = exp[3]
     first_arg = exp.first_arg
 
     #See if it is possible to simplify some basic cases
@@ -154,7 +151,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
         temp_exp = process_array_access target, exp.args
         exp = temp_exp if temp_exp
       elsif hash? target
-        temp_exp = process_hash_access target, exp.args
+        temp_exp = process_hash_access target, first_arg
         exp = temp_exp if temp_exp
       end
     when :merge!, :update
@@ -204,7 +201,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   def process_methdef exp
     env.scope do
       set_env_defaults
-      process exp.body
+      process_all exp.body
     end
     exp
   end
@@ -213,7 +210,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   def process_selfdef exp
     env.scope do
       set_env_defaults
-      process exp.body
+      process_all exp.body
     end
     exp
   end
@@ -231,8 +228,10 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
 
     if @inside_if and val = env[local]
       #avoid setting to value it already is (e.g. "1 or 1")
-      if val != exp.rhs and val[1] != exp.rhs and val[2] != exp.rhs
-        env[local] = Sexp.new(:or, val, exp.rhs).line(exp.line || -2)
+      if val != exp.rhs
+        unless node_type?(val, :or) and (val.rhs == exp.rhs or val.lhs == exp.rhs)
+          env[local] = Sexp.new(:or, val, exp.rhs).line(exp.line || -2)
+        end
       end
     else
       env[local] = exp.rhs
@@ -305,7 +304,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     if method == :[]=
       index = exp.first_arg = process(args.first)
       value = exp.second_arg = process(args.second)
-      match = Sexp.new(:call, target, :[], Sexp.new(:arglist, index))
+      match = Sexp.new(:call, target, :[], index)
       env[match] = value
 
       if hash? target
@@ -314,7 +313,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     elsif method.to_s[-1,1] == "="
       value = exp.first_arg = process(args.first)
       #This is what we'll replace with the value
-      match = Sexp.new(:call, target, method.to_s[0..-2].to_sym, Sexp.new(:arglist))
+      match = Sexp.new(:call, target, method.to_s[0..-2].to_sym)
 
       if @inside_if and val = env[match]
         if val != value
@@ -336,7 +335,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     hash = hash.deep_clone
     hash_iterate args do |key, replacement|
       hash_insert hash, key, replacement
-      match = Sexp.new(:call, hash, :[], Sexp.new(:arglist, key))
+      match = Sexp.new(:call, hash, :[], key)
       env[match] = replacement
     end
     hash
@@ -361,7 +360,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     target = exp[1] = process(exp[1])
     index = exp[2][1] = process(exp[2][1])
     value = exp[4] = process(exp[4])
-    match = Sexp.new(:call, target, :[], Sexp.new(:arglist, index))
+    match = Sexp.new(:call, target, :[], index)
 
     unless env[match]
       if request_value? target
@@ -383,7 +382,7 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     value = exp[4] = process(exp[4])
     method = exp[2]
 
-    match = Sexp.new(:call, target, method.to_s[0..-2].to_sym, Sexp.new(:arglist))
+    match = Sexp.new(:call, target, method.to_s[0..-2].to_sym)
 
     unless env[match]
       env[match] = value
@@ -392,8 +391,10 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     exp
   end
 
+  #This is the right hand side value of a multiple assignment,
+  #like `x = y, z`
   def process_svalue exp
-    exp[1]
+    exp.value
   end
 
   #Constant assignments like
@@ -423,9 +424,9 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
     if true? condition
       exps = [exp.then_clause]
     elsif false? condition
-      exps = exp[3..-1]
+      exps = [exp.else_clause]
     else
-      exps = exp[2..-1]
+      exps = [exp.then_clause, exp.else_clause]
     end
 
     was_inside = @inside_if
@@ -461,15 +462,9 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   end
 
   #Process hash access by returning the value associated
-  #with the given arguments.
-  def process_hash_access target, args
-    if args.length == 1
-      index = args[0]
-
-      hash_access(target, index)
-    else
-      nil
-    end
+  #with the given argument.
+  def process_hash_access target, index
+    hash_access(target, index)
   end
 
   #Join two array literals into one.
@@ -482,8 +477,9 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
   #Join two string literals into one.
   def join_strings string1, string2
     result = Sexp.new(:str)
-    result[1] = string1[1] + string2[1]
-    if result[1].length > 50
+    result.value = string1.value + string2.value
+
+    if result.value.length > 50
       string1
     else
       result
@@ -530,8 +526,8 @@ class Brakeman::AliasProcessor < Brakeman::SexpProcessor
 
   #Finds the inner most call target which is not the target of a call to <<
   def find_push_target exp
-    if call? exp and exp[2] == :<<
-      find_push_target exp[1]
+    if call? exp and exp.method == :<<
+      find_push_target exp.target
     else
       exp
     end

lib/brakeman/processors/base_processor.rb

@@ -20,24 +20,14 @@ class Brakeman::BaseProcessor < Brakeman::SexpProcessor
   def process_class exp
     current_class = @current_class
     @current_class = class_name exp[1]
-    process exp[3]
+    process_all exp.body
     @current_class = current_class
     exp
   end
 
   #Process a new scope. Removes expressions that are set to nil.
   def process_scope exp
-    exp = exp.dup
-    exp.shift
-    exp.map! do |e|
-      res = process e
-      if res.empty?
-        res = nil
-      else
-        res
-      end
-    end.compact
-    exp.unshift :scope
+    #NOPE?
   end
 
   #Default processing.

lib/brakeman/processors/controller_alias_processor.rb

@@ -98,7 +98,7 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor
         end
       end
 
-      process exp.body
+      process_all exp.body
 
       if is_route and not @rendered
         process_default_render exp
@@ -149,7 +149,7 @@ class Brakeman::ControllerAliasProcessor < Brakeman::AliasProcessor
       end
     else
       processor = Brakeman::AliasProcessor.new @tracker
-      processor.process_safely(method.body)
+      processor.process_safely(method.body_list)
 
       ivars = processor.only_ivars(:include_request_vars).all
 

lib/brakeman/processors/controller_processor.rb

@@ -2,7 +2,7 @@ require 'brakeman/processors/base_processor'
 
 #Processes controller. Results are put in tracker.controllers
 class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
-  FORMAT_HTML = Sexp.new(:call, Sexp.new(:lvar, :format), :html, Sexp.new(:arglist))
+  FORMAT_HTML = Sexp.new(:call, Sexp.new(:lvar, :format), :html)
 
   def initialize app_tree, tracker
     super(tracker)
@@ -50,7 +50,7 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
                     :src => exp,
                     :file => @file_name }
     @tracker.controllers[@controller[:name]] = @controller
-    exp.body = process exp.body
+    exp.body = process_all! exp.body
     set_layout_name
     @controller = nil
     exp
@@ -117,7 +117,7 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
       call.line(exp.line)
       call
     else
-      call = Sexp.new :call, target, method, process(exp.arglist) #RP 3 TODO
+      call = make_call target, method, process_all!(exp.args)
       call.line(exp.line)
       call
     end
@@ -127,11 +127,10 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
   def process_defn exp
     name = exp.method_name
     @current_method = name
-    res = Sexp.new :methdef, name, process(exp[2]), process(exp.body.block)
+    res = Sexp.new :methdef, name, exp.formal_args, *process_all!(exp.body)
     res.line(exp.line)
     @current_method = nil
     @controller[@visibility][name] = res unless @controller.nil?
-
     res
   end
 
@@ -152,7 +151,7 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
     end
 
     @current_method = name
-    res = Sexp.new :selfdef, target, name, process(exp[3]), process(exp.body.block)
+    res = Sexp.new :selfdef, target, name, exp.formal_args, *process_all!(exp.body)
     res.line(exp.line)
     @current_method = nil
     @controller[@visibility][name] = res unless @controller.nil?
@@ -189,9 +188,9 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
     filter_name = ("fake_filter" + rand.to_s[/\d+$/]).to_sym
     args = exp.block_call.arglist
     args.insert(1, Sexp.new(:lit, filter_name))
-    before_filter_call = Sexp.new(:call, nil, :before_filter, args)
+    before_filter_call = make_call(nil, :before_filter, args)
 
-    if exp.block_args
+    if exp.block_args.length > 1
       block_variable = exp.block_args[1]
     else
       block_variable = :temp
@@ -204,12 +203,11 @@ class Brakeman::ControllerProcessor < Brakeman::BaseProcessor
     end
 
     #Build Sexp for filter method
-    body = Sexp.new(:scope,
-            Sexp.new(:block,
-              Sexp.new(:lasgn, block_variable,
-                Sexp.new(:call, Sexp.new(:const, @controller[:name]), :new, Sexp.new(:arglist)))).concat(block_inner))
+    body = Sexp.new(:lasgn,
+                    block_variable, 
+                    Sexp.new(:call, Sexp.new(:const, @controller[:name]), :new))
 
-    filter_method = Sexp.new(:defn, filter_name, Sexp.new(:args), body).line(exp.line)
+    filter_method = Sexp.new(:defn, filter_name, Sexp.new(:args), body).concat(block_inner).line(exp.line)
 
     vis = @visibility
     @visibility = :private

lib/brakeman/processors/erb_template_processor.rb

@@ -4,7 +4,7 @@ require 'brakeman/processors/template_processor'
 #(those ending in .html.erb or .rthml).
 class Brakeman::ErbTemplateProcessor < Brakeman::TemplateProcessor
   
-  #s(:call, TARGET, :method, s(:arglist))
+  #s(:call, TARGET, :method, ARGS)
   def process_call exp
     target = exp.target
     if sexp? target
@@ -16,14 +16,14 @@ class Brakeman::ErbTemplateProcessor < Brakeman::TemplateProcessor
     if node_type? target, :lvar and target.value == :_erbout
       if method == :concat
         @inside_concat = true
-        args = exp.arglist = process(exp.arglist)
+        exp.arglist = process(exp.arglist)
         @inside_concat = false
 
-        if args.length > 2
+        if exp.args.length > 2
           raise Exception.new("Did not expect more than a single argument to _erbout.concat")
         end
 
-        arg = args[1]
+        arg = exp.first_arg
 
         if arg.node_type == :call and arg.method == :to_s #erb always calls to_s on output
           arg = arg.target
@@ -47,8 +47,7 @@ class Brakeman::ErbTemplateProcessor < Brakeman::TemplateProcessor
       make_render_in_view exp
     else
       #TODO: Is it really necessary to create a new Sexp here?
-      args = exp.arglist = process(exp.arglist)
-      call = Sexp.new :call, target, method, args
+      call = make_call target, method, process_all!(exp.args)
       call.original_line(exp.original_line)
       call.line(exp.line)
       call
@@ -64,7 +63,7 @@ class Brakeman::ErbTemplateProcessor < Brakeman::TemplateProcessor
         process e
       end
       @inside_concat = true
-      process exp[-1]
+      process exp.last
     else
       exp.map! do |e|
         res = process e

lib/brakeman/processors/erubis_template_processor.rb

@@ -3,7 +3,7 @@ require 'brakeman/processors/template_processor'
 #Processes ERB templates using Erubis instead of erb.
 class Brakeman::ErubisTemplateProcessor < Brakeman::TemplateProcessor
   
-  #s(:call, TARGET, :method, s(:arglist))
+  #s(:call, TARGET, :method, ARGS)
   def process_call exp
     target = exp.target
     if sexp? target
@@ -46,8 +46,7 @@ class Brakeman::ErubisTemplateProcessor < Brakeman::TemplateProcessor
       make_render_in_view exp
     else
       #TODO: Is it really necessary to create a new Sexp here?
-      args = exp.arglist = process(exp.arglist)
-      call = Sexp.new :call, target, method, args
+      call = make_call target, method, process_all!(exp.args)
       call.original_line(exp.original_line)
       call.line(exp.line)
       call

lib/brakeman/processors/haml_template_processor.rb

@@ -95,8 +95,7 @@ class Brakeman::HamlTemplateProcessor < Brakeman::TemplateProcessor
       make_render_in_view exp
     else
       #TODO: Do we really need a new Sexp here?
-      args = process exp.arglist
-      call = Sexp.new :call, target, method, args
+      call = make_call target, method, process_all!(exp.args)
       call.original_line(exp.original_line)
       call.line(exp.line)
       call

lib/brakeman/processors/lib/find_all_calls.rb

@@ -22,12 +22,12 @@ class Brakeman::FindAllCalls < Brakeman::BaseProcessor
 
   #Process body of method
   def process_methdef exp
-    process exp.body
+    process_all exp.body
   end
 
   #Process body of method
   def process_selfdef exp
-    process exp.body
+    process_all exp.body
   end
 
   #Process body of block

lib/brakeman/processors/lib/find_call.rb

@@ -69,7 +69,7 @@ class Brakeman::FindCall < Brakeman::BaseProcessor
 
   #Process body of method
   def process_methdef exp
-    process exp.body
+    process_all exp.body
   end
 
   alias :process_selfdef :process_methdef

lib/brakeman/processors/lib/processor_helper.rb

@@ -7,6 +7,18 @@ module Brakeman::ProcessorHelper
     exp
   end
 
+  def process_all! exp
+    exp.map! do |e|
+      if sexp? e
+        process e
+      else
+        e
+      end
+    end
+
+    exp
+  end
+
   #Sets the current module.
   def process_module exp
     module_name = class_name(exp.class_name).to_s
@@ -18,7 +30,7 @@ module Brakeman::ProcessorHelper
       @current_module = module_name
     end
 
-    process exp.body
+    process_all exp.body
 
     @current_module = prev_module
 
@@ -35,7 +47,7 @@ module Brakeman::ProcessorHelper
       when :lvar
         exp.value.to_sym
       when :colon2
-        "#{class_name(exp[1])}::#{exp[2]}".to_sym
+        "#{class_name(exp.lhs)}::#{exp.rhs}".to_sym
       when :colon3
         "::#{exp.value}".to_sym
       when :call

lib/brakeman/processors/lib/rails2_config_processor.rb

-#Replace block variable in
-#
-#  Rails::Initializer.run |config|
-#
-#with this value so we can keep track of it.
-Brakeman::RAILS_CONFIG = Sexp.new(:const, :"!BRAKEMAN_RAILS_CONFIG") unless defined? Brakeman::RAILS_CONFIG
-
 #Processes configuration. Results are put in tracker.config.
 #
 #Configuration of Rails via Rails::Initializer are stored in tracker.config[:rails].
@@ -20,6 +13,13 @@ Brakeman::RAILS_CONFIG = Sexp.new(:const, :"!BRAKEMAN_RAILS_CONFIG") unless defi
 #
 #Values for tracker.config[:rails] will still be Sexps.
 class Brakeman::Rails2ConfigProcessor < Brakeman::BaseProcessor
+  #Replace block variable in
+  #
+  #  Rails::Initializer.run |config|
+  #
+  #with this value so we can keep track of it.
+  RAILS_CONFIG = Sexp.new(:const, :"!BRAKEMAN_RAILS_CONFIG")
+
   def initialize *args
     super
     @tracker.config[:rails] ||= {}
@@ -46,7 +46,7 @@ class Brakeman::Rails2ConfigProcessor < Brakeman::BaseProcessor
 
   #Look for configuration settings
   def process_attrasgn exp
-    if exp.target == Brakeman::RAILS_CONFIG
+    if exp.target == RAILS_CONFIG
       #Get rid of '=' at end
       attribute = exp.method.to_s[0..-2].to_sym
       if exp.args.length > 1
@@ -83,12 +83,12 @@ class Brakeman::Rails2ConfigProcessor < Brakeman::BaseProcessor
   def include_rails_config? exp
     target = exp.target
     if call? target
-      if target.target == Brakeman::RAILS_CONFIG
+      if target.target == RAILS_CONFIG
         true
       else
         include_rails_config? target
       end
-    elsif target == Brakeman::RAILS_CONFIG
+    elsif target == RAILS_CONFIG
       true
     else
       false
@@ -107,7 +107,7 @@ class Brakeman::Rails2ConfigProcessor < Brakeman::BaseProcessor
       attribute = exp.method.to_s[0..-2].to_sym
       get_rails_config(exp.target) << attribute
     elsif call? exp
-      if exp.target == Brakeman::RAILS_CONFIG
+      if exp.target == RAILS_CONFIG
         [exp.method]
       else
         get_rails_config(exp.target) << exp.method
@@ -129,14 +129,13 @@ class Brakeman::ConfigAliasProcessor < Brakeman::AliasProcessor
   #    ...
   #  end
   #
-  #and replace config with Brakeman::RAILS_CONFIG
+  #and replace config with RAILS_CONFIG
   def process_iter exp
     target = exp.block_call.target
     method = exp.block_call.method
 
-
     if sexp? target and target == RAILS_INIT and method == :run
-      exp.block_args.rhs = Brakeman::RAILS_CONFIG
+      env[Sexp.new(:lvar, exp.block_args.value)] = Brakeman::Rails2ConfigProcessor::RAILS_CONFIG
     end
 
     process_default exp

lib/brakeman/processors/lib/rails2_route_processor.rb

@@ -200,7 +200,12 @@ class Brakeman::Rails2RoutesProcessor < Brakeman::BaseProcessor
 
     exp.last.each_with_index do |e,i|
       if symbol? e and e.value == :action
-        @tracker.routes[@current_controller] << exp.last[i + 1].value.to_sym
+        action = exp.last[i + 1]
+        
+        if node_type? action, :lit
+          @tracker.routes[@current_controller] << action.value.to_sym
+        end
+
         return
       end
     end
@@ -211,7 +216,7 @@ class Brakeman::Rails2RoutesProcessor < Brakeman::BaseProcessor
   # end
   def process_with_options exp
     @with_options = exp.block_call.args.last
-    @nested = Sexp.new(:lvar, exp.block_args.lhs)
+    @nested = Sexp.new(:lvar, exp.block_args.value)
 
     self.current_controller = check_for_controller_name exp.block_call.args
     
@@ -233,7 +238,7 @@ class Brakeman::Rails2RoutesProcessor < Brakeman::BaseProcessor
     @prefix << camelize(call.first_arg.value)
 
     if formal_args
-      @nested = Sexp.new(:lvar, formal_args.lhs)
+      @nested = Sexp.new(:lvar, formal_args.value)
     end
 
     process block

lib/brakeman/processors/lib/rails3_config_processor.rb

-Brakeman::RAILS_CONFIG = Sexp.new(:call, nil, :config, Sexp.new(:arglist))
-  
 #Processes configuration. Results are put in tracker.config.
 #
 #Configuration of Rails via Rails::Initializer are stored in tracker.config[:rails].
@@ -15,6 +13,8 @@ Brakeman::RAILS_CONFIG = Sexp.new(:call, nil, :config, Sexp.new(:arglist))
 #
 #Values for tracker.config[:rails] will still be Sexps.
 class Brakeman::Rails3ConfigProcessor < Brakeman::BaseProcessor
+  RAILS_CONFIG = Sexp.new(:call, nil, :config)
+
   def initialize *args
     super
     @tracker.config[:rails] ||= {}
@@ -31,7 +31,7 @@ class Brakeman::Rails3ConfigProcessor < Brakeman::BaseProcessor
   def process_iter exp
     if node_type?(exp.block_call.target, :colon2) and exp.block_call.method == :Application
       @inside_config = true
-      process exp.body if sexp? exp.body
+      process_all exp.body if sexp? exp.body
       @inside_config = false
     end
 
@@ -42,7 +42,7 @@ class Brakeman::Rails3ConfigProcessor < Brakeman::BaseProcessor
   def process_class exp
     if exp.class_name == :Application
       @inside_config = true
-      process exp.body if sexp? exp.body
+      process_all exp.body if sexp? exp.body
       @inside_config = false
     end
 
@@ -53,7 +53,7 @@ class Brakeman::Rails3ConfigProcessor < Brakeman::BaseProcessor
   def process_attrasgn exp
     return exp unless @inside_config
 
-    if exp.target == Brakeman::RAILS_CONFIG
+    if exp.target == RAILS_CONFIG
       #Get rid of '=' at end
       attribute = exp.method.to_s[0..-2].to_sym
       if exp.args.length > 1
@@ -80,12 +80,12 @@ class Brakeman::Rails3ConfigProcessor < Brakeman::BaseProcessor
   def include_rails_config? exp
     target = exp.target
     if call? target
-      if target.target == Brakeman::RAILS_CONFIG
+      if target.target == RAILS_CONFIG
         true
       else
         include_rails_config? target
       end
-    elsif target == Brakeman::RAILS_CONFIG
+    elsif target == RAILS_CONFIG
       true
     else
       false
@@ -104,7 +104,7 @@ class Brakeman::Rails3ConfigProcessor < Brakeman::BaseProcessor
       attribute = exp.method.to_s[0..-2].to_sym
       get_rails_config(exp.target) << attribute
     elsif call? exp
-      if exp.target == Brakeman::RAILS_CONFIG
+      if exp.target == RAILS_CONFIG
         [exp.method]
       else
         get_rails_config(exp.target) << exp.method

lib/brakeman/processors/lib/rails3_route_processor.rb

@@ -252,8 +252,7 @@ class Brakeman::Rails3RoutesProcessor < Brakeman::BaseProcessor
   end
 
   def process_controller_block exp
-    args = exp[1][3]
-    self.current_controller = args[1][1]
+    self.current_controller = exp.block_call.first_arg.value
 
     in_controller_block do
       process exp[-1] if exp[-1]

lib/brakeman/processors/lib/render_helper.rb

@@ -92,7 +92,7 @@ module Brakeman::RenderHelper
 
       if hash? options[:locals]
         hash_iterate options[:locals] do |key, value|
-          template_env[Sexp.new(:call, nil, key.value, Sexp.new(:arglist))] = value
+          template_env[Sexp.new(:call, nil, key.value)] = value
         end
       end
 
@@ -111,7 +111,7 @@ module Brakeman::RenderHelper
 
         collection = get_class_target(options[:collection]) || Brakeman::Tracker::UNKNOWN_MODEL
 
-        template_env[Sexp.new(:call, nil, variable, Sexp.new(:arglist))] = Sexp.new(:call, Sexp.new(:const, collection), :new, Sexp.new(:arglist))
+        template_env[Sexp.new(:call, nil, variable)] = Sexp.new(:call, Sexp.new(:const, collection), :new)
       end
 
       #Set original_line for values so it is clear

lib/brakeman/processors/library_processor.rb

@@ -49,7 +49,7 @@ class Brakeman::LibraryProcessor < Brakeman::BaseProcessor
       @tracker.libs[name] = @current_class
     end
 
-    exp.body = process exp.body
+    exp.body = process_all! exp.body
 
     if outer_class
       @current_class = outer_class
@@ -86,7 +86,7 @@ class Brakeman::LibraryProcessor < Brakeman::BaseProcessor
       @tracker.libs[name] = @current_module
     end
 
-    exp.body = process exp.body
+    exp.body = process_all! exp.body
 
     if outer_class
       @current_module = outer_class

lib/brakeman/processors/model_processor.rb

@@ -19,7 +19,7 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
     process src
   end
 
-  #s(:class, NAME, PARENT, s(:scope ...))
+  #s(:class, NAME, PARENT, BODY)
   def process_class exp
     name = class_name exp.class_name
 
@@ -44,9 +44,9 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
         :associations => {},
         :file => @file_name }
       @tracker.models[@model[:name]] = @model
-      res = process exp.body
+      exp.body = process_all! exp.body
       @model = nil
-      res
+      exp
     end
   end
 
@@ -81,8 +81,12 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
         when :attr_accessible
           @model[:attr_accessible] ||= []
           args = args.map do |e|
-            e[1]
-          end
+            if node_type? e, :lit
+              e.value
+            else
+              nil
+            end
+          end.compact
 
           @model[:attr_accessible].concat args
         else
@@ -92,14 +96,14 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
               @model[:associations][method].concat exp.args
             else
               @model[:options][method] ||= []
-              @model[:options][method] << exp.arglist
+              @model[:options][method] << exp.arglist.line(exp.line)
             end
           end
         end
       end
       ignore
     else
-      call = Sexp.new :call, target, method, process(exp.arglist)
+      call = make_call target, method, process_all!(exp.args)
       call.line(exp.line)
       call
     end
@@ -111,7 +115,7 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
     name = exp.method_name
 
     @current_method = name
-    res = Sexp.new :methdef, name, exp[2], process(exp.body.value)
+    res = Sexp.new :methdef, name, exp.formal_args, *process_all!(exp.body)
     res.line(exp.line)
     @current_method = nil
     if @model
@@ -133,7 +137,7 @@ class Brakeman::ModelProcessor < Brakeman::BaseProcessor
     end
 
     @current_method = name
-    res = Sexp.new :selfdef, target, name, exp[3], process(exp.body.value)
+    res = Sexp.new :selfdef, target, name, exp.formal_args, *process_all!(exp.body)
     res.line(exp.line)
     @current_method = nil
     if @model

lib/brakeman/processors/output_processor.rb

@@ -14,6 +14,7 @@ class Brakeman::OutputProcessor < Ruby2Ruby
   end
 
   alias process_safely format
+  alias process_methdef process_defn
 
   def process exp
     begin
@@ -100,7 +101,7 @@ class Brakeman::OutputProcessor < Ruby2Ruby
 
   def process_call_with_block exp
     call = process exp[0]
-    block = process exp[1] if exp[1]
+    block = process_rlist exp[2..-1]
     out = "#{call} do\n #{block}\n end"
     exp.clear
     out

lib/brakeman/processors/template_alias_processor.rb

@@ -40,29 +40,29 @@ class Brakeman::TemplateAliasProcessor < Brakeman::AliasProcessor
   #Looks for form methods and iterating over collections of Models
   def process_call_with_block exp
     process_default exp
-    
+
     call = exp.block_call
 
     if call? call
       target = call.target
       method = call.method
-      args = exp.block_args
+      arg = exp.block_args.first_param
       block = exp.block
 
       #Check for e.g. Model.find.each do ... end
-      if method == :each and args and block and model = get_model_target(target)
-        if node_type? args, :lasgn
+      if method == :each and arg and block and model = get_model_target(target)
+        if arg.is_a? Symbol
           if model == target.target
-            env[Sexp.new(:lvar, args.lhs)] = Sexp.new(:call, model, :new, Sexp.new(:arglist))
+            env[Sexp.new(:lvar, arg)] = Sexp.new(:call, model, :new)
           else
-            env[Sexp.new(:lvar, args.lhs)] = Sexp.new(:call, Sexp.new(:const, Brakeman::Tracker::UNKNOWN_MODEL), :new, Sexp.new(:arglist))
+            env[Sexp.new(:lvar, arg)] = Sexp.new(:call, Sexp.new(:const, Brakeman::Tracker::UNKNOWN_MODEL), :new)
           end
 
           process block if sexp? block
         end
       elsif FORM_METHODS.include? method
-        if node_type? args, :lasgn
-          env[Sexp.new(:lvar, args.lhs)] = Sexp.new(:call, Sexp.new(:const, :FormBuilder), :new, Sexp.new(:arglist)) 
+        if arg.is_a? Symbol
+          env[Sexp.new(:lvar, arg)] = Sexp.new(:call, Sexp.new(:const, :FormBuilder), :new)
 
           process block if sexp? block
         end

lib/brakeman/scanner.rb

 require 'rubygems'
 begin
-  if RUBY_VERSION =~ /^1\.9/
-    #Load our own version of ruby_parser :'(
-    require 'ruby_parser/ruby_parser.rb'
-  else
-    require 'ruby_parser'
-    require 'ruby_parser/bm_sexp.rb'
-  end
-
+  require 'ruby_parser'
+  require 'ruby_parser/bm_sexp.rb'
   require 'ruby_parser/bm_sexp_processor.rb'
 
   require 'haml'
@@ -46,13 +40,8 @@ class Brakeman::Scanner
       Brakeman.notify "[Notice] Detected Rails 3 application"
     end
 
+    @ruby_parser = ::RubyParser
     @processor = processor || Brakeman::Processor.new(@app_tree, options)
-
-    if RUBY_1_9
-      @ruby_parser = ::Ruby19Parser
-    else
-      @ruby_parser = ::RubyParser
-    end
   end
 
   #Returns the Tracker generated from the scan

lib/brakeman/util.rb

@@ -4,21 +4,21 @@ require 'active_support/inflector'
 #This is a mixin containing utility methods.
 module Brakeman::Util
 
-  QUERY_PARAMETERS = Sexp.new(:call, Sexp.new(:call, nil, :request, Sexp.new(:arglist)), :query_parameters, Sexp.new(:arglist))
+  QUERY_PARAMETERS = Sexp.new(:call, Sexp.new(:call, nil, :request), :query_parameters)
 
-  PATH_PARAMETERS = Sexp.new(:call, Sexp.new(:call, nil, :request, Sexp.new(:arglist)), :path_parameters, Sexp.new(:arglist))
+  PATH_PARAMETERS = Sexp.new(:call, Sexp.new(:call, nil, :request), :path_parameters)
 
-  REQUEST_PARAMETERS = Sexp.new(:call, Sexp.new(:call, nil, :request, Sexp.new(:arglist)), :request_parameters, Sexp.new(:arglist))
+  REQUEST_PARAMETERS = Sexp.new(:call, Sexp.new(:call, nil, :request), :request_parameters)
 
-  REQUEST_PARAMS = Sexp.new(:call, Sexp.new(:call, nil, :request, Sexp.new(:arglist)), :parameters, Sexp.new(:arglist))
+  REQUEST_PARAMS = Sexp.new(:call, Sexp.new(:call, nil, :request), :parameters)
 
-  REQUEST_ENV = Sexp.new(:call, Sexp.new(:call, nil, :request, Sexp.new(:arglist)), :env, Sexp.new(:arglist))
+  REQUEST_ENV = Sexp.new(:call, Sexp.new(:call, nil, :request), :env)
 
-  PARAMETERS = Sexp.new(:call, nil, :params, Sexp.new(:arglist))
+  PARAMETERS = Sexp.new(:call, nil, :params)
 
-  COOKIES = Sexp.new(:call, nil, :cookies, Sexp.new(:arglist))
+  COOKIES = Sexp.new(:call, nil, :cookies)
 
-  SESSION = Sexp.new(:call, nil, :session, Sexp.new(:arglist))
+  SESSION = Sexp.new(:call, nil, :session)
 
   ALL_PARAMETERS = Set[PARAMETERS, QUERY_PARAMETERS, PATH_PARAMETERS, REQUEST_PARAMETERS, REQUEST_PARAMS]
 
@@ -239,6 +239,22 @@ module Brakeman::Util
     false
   end
 
+  def make_call target, method, *args
+    call = Sexp.new(:call, target, method)
+
+    if args.empty? or args.first.empty?
+      #nothing to do
+    elsif node_type? args.first, :arglist
+      call.concat args.first[1..-1]
+    elsif args.first.node_type.is_a? Sexp #just a list of args
+      call.concat args.first
+    else
+      call.concat args
+    end
+
+    call
+  end
+
   #Return file name related to given warning. Uses +warning.file+ if it exists
   def file_for warning, tracker = nil
     if tracker.nil?

lib/ruby_parser/bm_sexp.rb

@@ -3,7 +3,7 @@
 #of a Sexp.
 class Sexp
   attr_reader :paren
-  ASSIGNMENT_BOOL = [:gasgn, :iasgn, :lasgn, :cvdecl, :cdecl, :or, :and]
+  ASSIGNMENT_BOOL = [:gasgn, :iasgn, :lasgn, :cvdecl, :cdecl, :or, :and, :colon2]
 
   def method_missing name, *args
     #Brakeman does not use this functionality,
@@ -23,6 +23,12 @@ class Sexp
     last
   end
 
+  def value= exp
+    raise WrongSexpError, "Sexp#value= called on multi-item Sexp", caller[1..-1] if size > 2
+    @my_hash_value = nil
+    self[1] = exp
+  end
+
   def second
     self[1]
   end
@@ -35,26 +41,6 @@ class Sexp
     self[0] = type
   end
 
-  #Don't use this, please.
-  #:nodoc:
-  def resbody delete = false
-    #RubyParser and Ruby2Ruby rely on method_missing for this, but since we
-    #don't want to use method_missing, here's a real method.
-    find_node :resbody, delete
-  end
-
-  #Don't use this, please.
-  #:nodoc:
-  def lasgn delete = false
-    find_node :lasgn, delete
-  end
-
-  #Don't use this, please.
-  #:nodoc:
-  def iasgn delete = false
-    find_node :iasgn, delete
-  end
-
   alias :node_type :sexp_type
   alias :values :sexp_body # TODO: retire
 
@@ -174,8 +160,19 @@ class Sexp
   #Sets the arglist in a method call.
   def arglist= exp
     expect :call, :attrasgn
-    self[3] = exp
-    #RP 3 TODO
+    start_index = 3
+
+    if exp.is_a? Sexp and exp.node_type == :arglist
+      exp = exp[1..-1]
+    end
+
+    exp.each_with_index do |e, i|
+      self[start_index + i] = e
+    end
+  end
+
+  def set_args *exp
+    self.arglist = exp
   end
 
   #Returns arglist for method call. This differs from Sexp#args, as Sexp#args
@@ -189,17 +186,14 @@ class Sexp
 
     case self.node_type
     when :call, :attrasgn
-      self[3]
+      self[3..-1].unshift :arglist
     when :super, :zsuper
       if self[1]
-        Sexp.new(:arglist).concat self[1..-1]
+        self[1..-1].unshift :arglist
       else
         Sexp.new(:arglist)
       end
     end
-
-    #For new ruby_parser
-    #Sexp.new(:arglist, *self[3..-1])
   end
 
   #Returns arguments of a method call. This will be an 'untyped' Sexp.
@@ -208,26 +202,19 @@ class Sexp
   #                                                             ^--------args--------^
   def args
     expect :call, :attrasgn, :super, :zsuper
-    #For new ruby_parser
-    #if self[3]
-    #  self[3..-1]
-    #else
-    #  []
-    #end
 
     case self.node_type
     when :call, :attrasgn
-      #For old ruby_parser
       if self[3]
-        self[3][1..-1]
+        self[3..-1]
       else
-        []
+        Sexp.new
       end
     when :super, :zsuper
       if self[1]
         self[1..-1]
       else
-        []
+        Sexp.new
       end
     end
   end
@@ -235,33 +222,25 @@ class Sexp
   #Returns first argument of a method call.
   def first_arg
     expect :call, :attrasgn
-    if self[3]
-      self[3][1]
-    end
+    self[3]
   end
 
   #Sets first argument of a method call.
   def first_arg= exp
     expect :call, :attrasgn
-    if self[3]
-      self[3][1] = exp
-    end
+    self[3] = exp
   end
 
   #Returns second argument of a method call.
   def second_arg
     expect :call, :attrasgn
-    if self[3]
-      self[3][2]
-    end
+    self[4]
   end
 
   #Sets second argument of a method call.
   def second_arg= exp
     expect :call, :attrasgn
-    if self[3]
-      self[3][2] = exp
-    end
+    self[4] = exp
   end
 
   #Returns condition of an if expression:
@@ -317,7 +296,11 @@ class Sexp
   #      s(:lasgn, :y),
   #       s(:block, s(:lvar, :y), s(:call, nil, :z, s(:arglist))))
   #       ^-------------------- block --------------------------^
-  def block
+  def block delete = nil
+    unless delete.nil? #this is from RubyParser
+      return find_node :block, delete
+    end
+
     expect :iter, :call_with_block, :scope, :resbody
 
     case self.node_type
@@ -342,6 +325,11 @@ class Sexp
     self[2]
   end
 
+  def first_param
+    expect :args
+    self[1]
+  end
+
   #Returns the left hand side of assignment or boolean:
   #
   #    s(:lasgn, :x, s(:lit, 1))
@@ -384,34 +372,61 @@ class Sexp
     end
   end
 
-  #Sets body
+  def formal_args
+    expect :defn, :defs, :methdef, :selfdef
+
+    case self.node_type
+    when :defn, :methdef
+      self[2]
+    when :defs, :selfdef
+      self[3]
+    end
+  end
+
+  #Sets body, which is now a complicated process because the body is no longer
+  #a separate Sexp, but just a list of Sexps.
   def body= exp
     expect :defn, :defs, :methdef, :selfdef, :class, :module
 
     case self.node_type
     when :defn, :methdef, :class
-      self[3] = exp
+      index = 3
     when :defs, :selfdef
-      self[4] = exp
+      index = 4
     when :module
-      self[2] = exp
+      index = 2
+    end
+
+    self.slice!(index..-1) #Remove old body
+
+    #Insert new body
+    exp.each do |e|
+      self[index] = e
+      index += 1
     end
   end
 
   #Returns body of a method definition, class, or module.
+  #This will be an untyped Sexp containing a list of Sexps from the body.
   def body
     expect :defn, :defs, :methdef, :selfdef, :class, :module
 
     case self.node_type
     when :defn, :methdef, :class
-      self[3]
+      self[3..-1]
     when :defs, :selfdef
-      self[4]
+      self[4..-1]
     when :module
-      self[2]
+      self[2..-1]
     end
   end
 
+  #Like Sexp#body, except the returned Sexp is of type :rlist
+  #instead of untyped.
+  def body_list
+    self.body.unshift :rlist
+  end
+
   def render_type
     expect :render
     self[1]
@@ -445,4 +460,17 @@ end
     RUBY
 end
 
+#Methods used by RubyParser which would normally go through method_missing but
+#we don't want that to happen because it hides Brakeman errors
+[:resbody, :lasgn, :iasgn, :splat].each do |method|
+  Sexp.class_eval <<-RUBY
+    def #{method} delete = false
+      if delete
+        @my_hash_value = false
+      end
+      find_node :#{method}, delete
+    end
+  RUBY
+end
+
 class WrongSexpError < RuntimeError; end

lib/ruby_parser/bm_sexp_processor.rb

@@ -61,8 +61,7 @@ class Brakeman::SexpProcessor
     result = nil
 
     type = exp.first
-    raise "type should be a Symbol, not: #{exp.first.inspect}" unless
-      Symbol === type
+    raise "Type should be a Symbol, not: #{exp.first.inspect} in #{exp.inspect}" unless Symbol === type
 
     in_context type do
       # now do a pass with the real processor (or generic)

lib/ruby_parser/ruby_parser.rb

-$LOAD_PATH.unshift File.dirname(File.expand_path(__FILE__))
-
-require 'ruby18_parser'
-require 'ruby19_parser'
-require 'ruby_parser_extras'

test/apps/rails2/config/routes.rb

@@ -54,4 +54,6 @@ ActionController::Routing::Routes.draw do |map|
   map.connect ':controller/:action/:id'
   map.connect ':controller/:action/:id.:format'
   map.some_routes
+
+  map.things "/things", :controller => "home", :action => "index_#{random_dynamic_thing}" 
 end

test/test.rb

@@ -111,13 +111,13 @@ module BrakemanTester::RescanTestHelper
   #given in `changed`.
   #
   #Provide an array of changed files for rescanning.
-  def before_rescan_of changed
+  def before_rescan_of changed, app = "rails3.2"
     changed = [changed] unless changed.is_a? Array
 
     Dir.mktmpdir do |dir|
       @dir = dir
 
-      FileUtils.cp_r "#{TEST_PATH}/apps/rails3.2/.", dir
+      FileUtils.cp_r "#{TEST_PATH}/apps/#{app}/.", dir
       @original = Brakeman.run :app_path => dir, :debug => false
 
       yield dir if block_given?
@@ -219,18 +219,14 @@ module BrakemanTester::RescanTestHelper
     replace_with_sexp file do |parsed|
       class_body = parsed.body
 
-      if class_body[1].node_type == :block
-        class_body[1].reject! do |node|
-          node.is_a? Sexp and
-          node.node_type == :defn and
-          node.method_name == method_name
-        end
-      elsif class_body[1].node_type == :defn and
-        class_body[1].method_name == method_name
-
-        class_body.delete_at 1
+      class_body.reject! do |node|
+        node.is_a? Sexp and
+        node.node_type == :defn and
+        node.method_name == method_name
       end
 
+      parsed.body = class_body
+
       parsed
     end
   end
@@ -239,28 +235,14 @@ module BrakemanTester::RescanTestHelper
     parsed_method = parse code
 
     replace_with_sexp file do |parsed|
-      class_body = parsed.body
-
-      if class_body[1].node_type == :block
-        class_body[1] << parsed_method
-      elsif class_body[1]
-        class_body[1] = s(:block,
-                          class_body[1],
-                          parsed_method)
-      else
-        class_body[1] = parsed_method
-      end
+      parsed.body = parsed.body << parsed_method
 
       parsed
     end
   end
 
   def parse code
-    if RUBY_VERSION =~ /^1\.9/
-      Ruby19Parser.new.parse code
-    else
-      RubyParser.new.parse code
-    end
+    RubyParser.new.parse code
   end
 end
 

test/tests/test_alias_processor.rb

 class AliasProcessorTests < Test::Unit::TestCase
   def assert_alias expected, original
-    if RUBY_VERSION =~ /^1\.9/
-      original_sexp = Ruby19Parser.new.parse original
-      expected_sexp = Ruby19Parser.new.parse expected
-    else
-      original_sexp = RubyParser.new.parse original
-      expected_sexp = RubyParser.new.parse expected
-    end
+    original_sexp = RubyParser.new.parse original
+    expected_sexp = RubyParser.new.parse expected
 
     processed_sexp = Brakeman::AliasProcessor.new.process_safely original_sexp
     result = processed_sexp.last

test/tests/test_brakeman.rb

 class UtilTests < Test::Unit::TestCase
   def setup
-    if RUBY_VERSION =~ /^1\.9/
-      @ruby_parser = Ruby19Parser
-    else
-      @ruby_parser = RubyParser
-    end
+    @ruby_parser = RubyParser
   end
 
   def util

test/tests/test_mass_assign_disable.rb

+class MassAssignDisableTest < Test::Unit::TestCase
+  include BrakemanTester::RescanTestHelper
+
+  def mass_assign_disable content
+    init = "config/initializers/mass_assign.rb"
+
+    before_rescan_of init, "rails2" do
+      write_file init, content
+    end
+
+    assert_changes
+    assert_fixed 3
+    assert_new 0
+  end
+
+  def test_disable_mass_assignment_by_send
+    mass_assign_disable "ActiveRecord::Base.send(:attr_accessible, nil)"
+  end
+
+  def test_disable_mass_assignment_by_module
+    mass_assign_disable <<-RUBY
+      module ActiveRecord
+        class Base
+          attr_accessible
+        end
+      end
+    RUBY
+  end
+
+  def test_disable_mass_assignment_by_module_and_nil
+    mass_assign_disable <<-RUBY
+      module ActiveRecord
+        class Base
+          attr_accessible nil
+        end
+      end
+    RUBY
+  end
+end

test/tests/test_output_processor.rb

@@ -140,7 +140,8 @@ class OutputProcessorTests < Test::Unit::TestCase
   def test_output_call_with_block
     assert_output "x do\n y\n end",
       Sexp.new(:call_with_block,
-               Sexp.new(:call, nil, :x, Sexp.new(:arglist)),
-               Sexp.new(:call, nil, :y, Sexp.new(:arglist)))
+               Sexp.new(:call, nil, :x),
+               Sexp.new(:args),
+               Sexp.new(:call, nil, :y))
   end
 end

test/tests/test_rails2.rb

@@ -107,7 +107,7 @@ class Rails2Tests < Test::Unit::TestCase
   def test_redirect
     assert_warning :type => :warning,
       :warning_type => "Redirect",
-      :line => 46,
+      :line => 45,
       :message => /^Possible unprotected redirect/,
       :confidence => 0,
       :file => /home_controller\.rb/
@@ -598,8 +598,8 @@ class Rails2Tests < Test::Unit::TestCase
   def test_explicit_render_template
     assert_warning :type => :template,
       :warning_type => "Cross Site Scripting",
-      :line => 2,
-      :message => /^Unescaped parameter value near line 2: params\[:ba/,
+      :line => 1,
+      :message => /^Unescaped parameter value near line 1: params\[:ba/,
       :confidence => 0,
       :file => /home\/test_render_template\.html\.haml/
   end

test/tests/test_rails3.rb

@@ -138,7 +138,7 @@ class Rails3Tests < Test::Unit::TestCase
   def test_redirect
     assert_warning :type => :warning,
       :warning_type => "Redirect",
-      :line => 46,
+      :line => 45,
       :message => /^Possible unprotected redirect near line /,
       :confidence => 0,
       :file => /home_controller\.rb/

test/tests/test_rails31.rb

@@ -485,7 +485,7 @@ class Rails31Tests < Test::Unit::TestCase
   def test_cross_site_request_forgery
     assert_warning :type => :warning,
       :warning_type => "Cross-Site Request Forgery",
-      :line => 93,
+      :line => 91,
       :message => /^Use\ whitelist\ \(:only\ =>\ \[\.\.\]\)\ when\ skipp/,
       :confidence => 1,
       :file => /users_controller\.rb/

test/tests/test_rails32.rb

@@ -25,7 +25,7 @@ class Rails32Tests < Test::Unit::TestCase
   def test_redirect_1
     assert_warning :type => :warning,
       :warning_type => "Redirect",
-      :line => 13,
+      :line => 14,
       :message => /^Possible\ unprotected\ redirect/,
       :confidence => 0,
       :file => /removal_controller\.rb/

test/tests/test_sexp.rb

@@ -2,11 +2,7 @@ require 'brakeman/processors/base_processor'
 
 class SexpTests < Test::Unit::TestCase
   def setup
-    if RUBY_VERSION[/^1\.9/]
-      @ruby_parser = ::Ruby19Parser
-    else
-      @ruby_parser = ::RubyParser
-    end
+    @ruby_parser = ::RubyParser
   end
 
   def parse string
@@ -16,7 +12,7 @@ class SexpTests < Test::Unit::TestCase
   def test_method_call_with_no_args
     exp = parse "x.y"
 
-    assert_equal s(:call, nil, :x, s(:arglist)), exp.target
+    assert_equal s(:call, nil, :x), exp.target
     assert_equal :y, exp.method
     assert_equal s(), exp.args
     assert_equal s(:arglist), exp.arglist
@@ -27,7 +23,7 @@ class SexpTests < Test::Unit::TestCase
   def test_method_call_with_args
     exp = parse 'x.y(1, 2, 3)'
 
-    assert_equal s(:call, nil, :x, s(:arglist)), exp.target
+    assert_equal s(:call, nil, :x), exp.target
     assert_equal :y, exp.method
     assert_equal s(s(:lit, 1), s(:lit, 2), s(:lit, 3)), exp.args
     assert_equal s(:arglist, s(:lit, 1), s(:lit, 2), s(:lit, 3)), exp.arglist
@@ -53,7 +49,7 @@ class SexpTests < Test::Unit::TestCase
     assert_equal :z, exp.target
   end
 
-  def test_method_call_set_args
+  def test_method_call_set_arglist
     exp = parse 'x.y'
     exp.arglist = s(:arglist, s(:lit, 1), s(:lit, 2))
 
@@ -63,15 +59,27 @@ class SexpTests < Test::Unit::TestCase
     assert_equal s(s(:lit, 1), s(:lit, 2)), exp.args
   end
 
+  def test_method_call_set_args
+    exp = parse "x.y"
+
+    assert_equal s(), exp.args
+
+    exp.set_args s(:lit, 1), s(:lit, 2)
+
+    assert_equal s(s(:lit, 1), s(:lit, 2)), exp.args
+    assert_equal s(:lit,1), exp.first_arg
+    assert_equal s(:lit, 2), exp.second_arg
+  end
+
   def test_method_call_with_block
     exp = parse "x do |z|; blah z; end"
     block = exp.block
     call = exp.block_call
     args = exp.block_args
 
-    assert_equal s(:call, nil, :x, s(:arglist)), call
-    assert_equal s(:lasgn, :z), args
-    assert_equal s(:call, nil, :blah, s(:arglist, s(:lvar, :z))), block
+    assert_equal s(:call, nil, :x), call
+    assert_equal s(:args, :z), args
+    assert_equal s(:call, nil, :blah, s(:lvar, :z)), block
   end
 
   def test_or
@@ -97,9 +105,9 @@ class SexpTests < Test::Unit::TestCase
     end
     RUBY
 
-    assert_equal s(:call, nil, :x, s(:arglist)), exp.condition
-    assert_equal s(:call, nil, :y, s(:arglist)), exp.then_clause
-    assert_equal s(:call, nil, :z, s(:arglist)), exp.else_clause
+    assert_equal s(:call, nil, :x), exp.condition
+    assert_equal s(:call, nil, :y), exp.then_clause
+    assert_equal s(:call, nil, :z), exp.else_clause
   end
 
   def test_local_assignment
@@ -167,7 +175,7 @@ class SexpTests < Test::Unit::TestCase
     end
     RUBY
 
-    assert_equal s(:scope, s(:rlist, s(:call, nil, :z, s(:arglist)), s(:lvar, :y))), exp.body
+    assert_equal s(s(:call, nil, :z), s(:lvar, :y)), exp.body
   end
 
   def test_method_def_body_single_line
@@ -177,7 +185,7 @@ class SexpTests < Test::Unit::TestCase
     end
     RUBY
 
-    assert_equal s(:scope, s(:rlist, s(:lvar, :y))), exp.body
+    assert_equal s(s(:lvar, :y)), exp.body
   end
 
   def test_class_body
@@ -188,7 +196,7 @@ class SexpTests < Test::Unit::TestCase
     end
     RUBY
 
-    assert_equal s(:scope, s(:defn, :y, s(:args), s(:scope, s(:block, s(:nil))))), exp.body
+    assert_equal s(s(:defn, :y, s(:args), s(:nil))), exp.body
   end
 
   def test_module_body
@@ -199,7 +207,7 @@ class SexpTests < Test::Unit::TestCase
     end
     RUBY
 
-    assert_equal s(:scope, s(:defn, :y, s(:args), s(:scope, s(:block, s(:nil))))), exp.body
+    assert_equal s(s(:defn, :y, s(:args), s(:nil))), exp.body
   end
 
   def test_class_name
@@ -233,7 +241,7 @@ class SexpTests < Test::Unit::TestCase
 
     assert_equal :super, exp.method
     assert_equal s(:arglist), exp.arglist
-    assert_equal [], exp.args
+    assert_equal s(), exp.args
   end
 
   def test_super_call