Merge pull request #540 from presidentbeef/add_check_for_CVE-2014-3415

Add check for CVE-2014-3415

lib/brakeman/checks/check_create_with.rb

+require 'brakeman/checks/base_check'
+
+class Brakeman::CheckCreateWith < Brakeman::BaseCheck
+  Brakeman::Checks.add self
+
+  @description = "Checks for strong params bypass in CVE-2014-3514"
+
+  def run_check
+    @warned = false
+
+    if version_between? "4.0.0", "4.0.8"
+      suggested_version = "4.0.9"
+    elsif version_between? "4.1.0", "4.1.4"
+      suggested_version = "4.1.5"
+    else
+      return
+    end
+
+    @message = "create_with is vulnerable to strong params bypass. Upgrade to Rails #{suggested_version} or patch"
+
+    tracker.find_call(:method => :create_with, :nested => true).each do |result|
+      process_result result
+    end
+
+    generic_warning unless @warned
+  end
+
+  def process_result result
+    return if duplicate? result
+    add_result result
+    arg = result[:call].first_arg
+
+    confidence = danger_level arg
+
+    if confidence
+      @warned = true
+
+      warn :warning_type => "Mass Assignment",
+        :warning_code => :CVE_2014_3514_call,
+        :result => result,
+        :message => @message,
+        :confidence => confidence,
+        :link_path => "https://groups.google.com/d/msg/rubyonrails-security/M4chq5Sb540/CC1Fh0Y_NWwJ"
+    end
+  end
+
+  #For a given create_with call, set confidence level.
+  #Ignore calls that use permit()
+  def danger_level exp
+    return unless sexp? exp
+
+    if call? exp and exp.method == :permit
+      nil
+    elsif request_value? exp
+      CONFIDENCE[:high]
+    elsif hash? exp
+      nil
+    elsif has_immediate_user_input?(exp)
+      CONFIDENCE[:high]
+    elsif include_user_input? exp
+      CONFIDENCE[:med]
+    else
+      CONFIDENCE[:low]
+    end
+  end
+
+  def generic_warning
+      warn :warning_type => "Mass Assignment",
+        :warning_code => :CVE_2014_3514,
+        :message => @message,
+        :file => gemfile_or_environment,
+        :confidence => CONFIDENCE[:med],
+        :link_path => "https://groups.google.com/d/msg/rubyonrails-security/M4chq5Sb540/CC1Fh0Y_NWwJ"
+  end
+end

lib/brakeman/warning_codes.rb

@@ -81,6 +81,8 @@ module Brakeman::WarningCodes
     :CVE_2014_0130 => 77,
     :CVE_2014_3482 => 78,
     :CVE_2014_3483 => 79,
+    :CVE_2014_3514 => 80,
+    :CVE_2014_3514_call => 81,
   }
 
   def self.code name

test/apps/rails4/app/controllers/users_controller.rb

@@ -52,4 +52,13 @@ class UsersController < ApplicationController
     params[:controller].to_sym
     params[:action].intern
   end
+
+  def mass_assignment_bypass
+    User.create_with(params)  # high warning
+    User.create_with(params).create # high warning
+    User.create_with(params[:x].permit(:y)) # should not warn, workaround
+    something.create_with({}) # should not warn on hash literals
+    x.create_with(y(params))  # medium warning
+    y.create_with(x)          # weak warning
+  end
 end

test/tests/brakeman.rb

@@ -19,6 +19,16 @@ class BrakemanTests < Test::Unit::TestCase
     assert_equal absolute_path, at.root
     assert_equal File.join(absolute_path, "Gemfile"), at.expand_path("Gemfile")
   end
+
+  def test_relative_path_in_warnings
+    relative_path = Pathname.new(File.dirname(__FILE__)).relative_path_from(Pathname.getwd)
+    absolute_path = relative_path.realpath.to_s
+    input = ["-p", relative_path.to_s]
+    options, _ = Brakeman::Options.parse input
+    at = Brakeman::AppTree.from_options options
+
+
+  end
 end
 
 class UtilTests < Test::Unit::TestCase

test/tests/rails4.rb

@@ -16,7 +16,7 @@ class Rails4Tests < Test::Unit::TestCase
       :controller => 0,
       :model => 1,
       :template => 2,
-      :generic => 32
+      :generic => 36
     }
   end
 
@@ -535,6 +535,49 @@ class Rails4Tests < Test::Unit::TestCase
       :user_input => nil
   end
 
+  def test_mass_assignment_CVE_2014_3415
+    assert_warning :type => :warning,
+      :warning_code => 81,
+      :fingerprint => "c4a619b7316e45a5927b098294ff39d7206f84bac084402630318bf6f89f396d",
+      :warning_type => "Mass Assignment",
+      :line => 57,
+      :message => /^create_with\ is\ vulnerable\ to\ strong\ para/,
+      :confidence => 0,
+      :relative_path => "app/controllers/users_controller.rb",
+      :user_input => nil
+
+    assert_warning :type => :warning,
+      :warning_code => 81,
+      :fingerprint => "c4a619b7316e45a5927b098294ff39d7206f84bac084402630318bf6f89f396d",
+      :warning_type => "Mass Assignment",
+      :line => 58,
+      :message => /^create_with\ is\ vulnerable\ to\ strong\ para/,
+      :confidence => 0,
+      :relative_path => "app/controllers/users_controller.rb",
+      :user_input => nil
+
+    assert_warning :type => :warning,
+      :warning_code => 81,
+      :fingerprint => "8c55b05e3467934ac900567d47b4ac496e9761424b66b246585d14ba5b2b0240",
+      :warning_type => "Mass Assignment",
+      :line => 61,
+      :message => /^create_with\ is\ vulnerable\ to\ strong\ para/,
+      :confidence => 1,
+      :relative_path => "app/controllers/users_controller.rb",
+      :user_input => nil
+
+    assert_warning :type => :warning,
+      :warning_code => 81,
+      :fingerprint => "aafdaf40064466b1eea16ca053072fb2ef20c999411108d606c8555ade2ce629",
+      :warning_type => "Mass Assignment",
+      :line => 62,
+      :message => /^create_with\ is\ vulnerable\ to\ strong\ para/,
+      :confidence => 2,
+      :relative_path => "app/controllers/users_controller.rb",
+      :user_input => nil
+
+  end
+
   def test_mass_assignment_with_permit!
     assert_warning :type => :warning,
       :warning_code => 70,

test/tests/rails4_with_engines.rb

@@ -11,7 +11,7 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
       :controller => 0,
       :model => 5,
       :template => 11,
-      :generic => 7 }
+      :generic => 8 }
   end
 
   def report
@@ -86,6 +86,18 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
       :user_input => nil
   end
 
+  def test_mass_assignment_CVE_2014_3415
+    assert_warning :type => :warning,
+      :warning_code => 80,
+      :fingerprint => "c3535608927977a6b2f7587021ce6c366895ec0637cf1c15988324349b22f76d",
+      :warning_type => "Mass Assignment",
+      :line => nil,
+      :message => /^create_with\ is\ vulnerable\ to\ strong\ para/,
+      :confidence => 1,
+      :relative_path => "Gemfile",
+      :user_input => nil
+  end
+
   def test_redirect_1
     assert_warning :type => :generic,
       :warning_code => 18,