Merge pull request #1089 from presidentbeef/fail_and_raise_are_early_…

…returns_too_i_guess

Treat fail/raise like early returns

lib/brakeman/processors/alias_processor.rb

@@ -706,9 +706,11 @@ def process_if exp
 
   def early_return? exp
     return true if node_type? exp, :return
+    return true if call? exp and [:fail, :raise].include? exp.method
 
     if node_type? exp, :block, :rlist
-      node_type? exp.last, :return
+      node_type? exp.last, :return or
+        (call? exp and [:fail, :raise].include? exp.method)
     else
       false
     end

test/apps/rails5/app/controllers/mixed_controller.rb

@@ -15,4 +15,10 @@ def another_early_return
 
     Statistics::AdminWithdrawal.send("export_#{bank_name}_#inc!")
   end
+
+  def yet_another_early_return
+    scope_name = params[:scope].presence
+    fail ActiveRecord::RecordNotFound unless ['safe', 'also_safe'].include?(scope_name)
+    Model.public_send(scope_name)
+  end
 end

test/tests/alias_processor.rb

@@ -883,6 +883,26 @@ def test_branch_array_include_return_more
     OUTPUT
   end
 
+  def test_branch_array_include_fail
+    assert_output <<-INPUT, <<-OUTPUT
+    fail unless ['a', 'b'].include? x
+    x
+    INPUT
+    fail unless ['a', 'b'].include? x
+    'a'
+    OUTPUT
+  end
+
+  def test_branch_array_include_raise
+    assert_output <<-INPUT, <<-OUTPUT
+    raise Z unless ['a', 'b'].include? x
+    x
+    INPUT
+    raise Z unless ['a', 'b'].include? x
+    'a'
+    OUTPUT
+  end
+
   def test_case_basic
     assert_output <<-INPUT, <<-OUTPUT
       z = 3

test/tests/rails5.rb

@@ -94,6 +94,19 @@ def test_dangerous_send_with_early_return
       :user_input => s(:call, s(:call, s(:params), :[], s(:lit, :filename)), :first)
   end
 
+  def test_dangerous_send_with_fail
+    assert_no_warning :type => :warning,
+      :warning_code => 23,
+      :fingerprint => "e39bb04370762208b68068f4dc823ec897b75bb50f4a5dee02e329e2b6dda733",
+      :warning_type => "Dangerous Send",
+      :line => 22,
+      :message => /^User\ controlled\ method\ execution/,
+      :confidence => 0,
+      :relative_path => "app/controllers/mixed_controller.rb",
+      :code => s(:call, s(:const, :Model), :public_send, s(:call, s(:call, s(:params), :[], s(:lit, :scope)), :presence)),
+      :user_input => s(:call, s(:call, s(:params), :[], s(:lit, :scope)), :presence)
+  end
+
   def test_no_symbol_denial_of_service
     assert_no_warning :type => :warning,
       :warning_code => 59,