Merge remote-tracking branch 'origin/Feb_18_CVEs'

Conflicts:
	CHANGES

CHANGES

@@ -5,6 +5,12 @@
  * Skip identically rendered templates
  * Fix HAML template processing
 
+# 2.4.1
+
+ * Add check for CVE-2014-0082
+ * Add check for CVE-2014-0081, replaces CVE-2013-6415
+ * Add check for CVE-2014-0080
+
 # 2.4.0 
 
  * Detect Rails LTS versions

lib/brakeman/checks/check_number_to_currency.rb

@@ -3,53 +3,64 @@ require 'brakeman/checks/base_check'
 class Brakeman::CheckNumberToCurrency < Brakeman::BaseCheck
   Brakeman::Checks.add self
 
-  @description = "Checks for number_to_currency XSS vulnerability in certain versions"
+  @description = "Checks for number helpers XSS vulnerabilities in certain versions"
 
   def run_check
-    return if lts_version? '2.3.18.6'
-
-    if (version_between? "2.0.0", "3.2.15" or version_between? "4.0.0", "4.0.1")
-      check_number_to_currency_usage
+    if version_between? "2.0.0", "2.3.18" or
+      version_between? "3.0.0", "3.2.16" or
+      version_between? "4.0.0", "4.0.2"
 
+      check_number_helper_usage
       generic_warning unless @found_any
     end
   end
 
   def generic_warning
-    message = "Rails #{tracker.config[:rails_version]} has a vulnerability in number_to_currency (CVE-2013-6415). Upgrade to Rails version "
+    message = "Rails #{tracker.config[:rails_version]} has a vulnerability in number helpers (CVE-2014-0081). Upgrade to Rails version "
 
-    if version_between? "2.3.0", "3.2.15"
-      message << "3.2.16"
+    if version_between? "2.3.0", "3.2.16"
+      message << "3.2.17"
     else
-      message << "4.0.2"
+      message << "4.0.3"
     end
 
     warn :warning_type => "Cross Site Scripting",
-      :warning_code => :CVE_2013_6415,
+      :warning_code => :CVE_2014_0081,
       :message => message,
       :confidence => CONFIDENCE[:med],
       :file => gemfile_or_environment,
       :link_path => "https://groups.google.com/d/msg/ruby-security-ann/9WiRn2nhfq0/2K2KRB4LwCMJ"
   end
 
-  def check_number_to_currency_usage
-    tracker.find_call(:target => false, :method => :number_to_currency).each do |result|
+  def check_number_helper_usage
+    number_methods = [:number_to_currency, :number_to_percentage, :number_to_human] 
+    tracker.find_call(:target => false, :methods => number_methods).each do |result|
       arg = result[:call].second_arg
       next unless arg
 
-      if match = (has_immediate_user_input? arg or has_immediate_model? arg)
-        match = match.match if match.is_a? Match
-        @found_any = true
-        warn_on_number_to_currency result, match
+      if not check_helper_option(result, arg) and hash? arg
+        hash_iterate(arg) do |key, value|
+          break if check_helper_option(result, value)
+        end
       end
     end
   end
 
-  def warn_on_number_to_currency result, match
+  def check_helper_option result, exp
+    if match = (has_immediate_user_input? exp or has_immediate_model? exp)
+      match = match.match if match.is_a? Match
+      warn_on_number_helper result, match
+      @found_any = true
+    else
+      false
+    end
+  end
+
+  def warn_on_number_helper result, match
     warn :result => result,
       :warning_type => "Cross Site Scripting",
-      :warning_code => :CVE_2013_6415_call,
-      :message => "Currency value in number_to_currency is not safe in Rails #{@tracker.config[:rails_version]}",
+      :warning_code => :CVE_2014_0081_call,
+      :message => "Format options in #{result[:call].method} are not safe in Rails #{@tracker.config[:rails_version]}",
       :confidence => CONFIDENCE[:high],
       :link_path => "https://groups.google.com/d/msg/ruby-security-ann/9WiRn2nhfq0/2K2KRB4LwCMJ",
       :user_input => match

lib/brakeman/checks/check_render_dos.rb

+require 'brakeman/checks/base_check'
+
+class Brakeman::CheckRenderDoS < Brakeman::BaseCheck
+  Brakeman::Checks.add self
+
+  @description = "Warn about denial of service with render :text (CVE-2014-0082)"
+
+  def run_check
+    if version_between? "3.0.0", "3.0.20" or
+       version_between? "3.1.0", "3.1.12" or
+       version_between? "3.2.0", "3.2.16"
+
+      tracker.find_call(:target => nil, :method => :render).each do |result|
+        if text_render? result
+          warn_about_text_render
+          break
+        end
+      end
+    end
+  end
+
+  def text_render? result
+    node_type? result[:call], :render and
+    result[:call].render_type == :text
+  end
+
+  def warn_about_text_render
+    message = "Rails #{tracker.config[:rails_version]} has a denial of service vulnerability (CVE-2014-0082). Upgrade to Rails version 3.2.17"
+
+    warn :warning_type => "Denial of Service",
+      :warning_code => :CVE_2014_0082,
+      :message => message,
+      :confidence => CONFIDENCE[:high],
+      :link_path => "https://groups.google.com/d/msg/rubyonrails-security/LMxO_3_eCuc/ozGBEhKaJbIJ",
+      :file => gemfile_or_environment
+  end
+end

lib/brakeman/checks/check_sql.rb

@@ -51,6 +51,8 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
 
     Brakeman.debug "Processing possible SQL calls"
     calls.each { |call| process_result call }
+
+    check_CVE_2014_0080
   end
 
   #Find calls to named_scope() or scope() in models
@@ -638,6 +640,19 @@ class Brakeman::CheckSQL < Brakeman::BaseCheck
     end
   end
 
+  # TODO: Move all SQL CVE checks to separate class
+  def check_CVE_2014_0080
+    return unless version_between? "4.0.0", "4.0.2" and
+                  @tracker.config[:gems].include? :pg
+
+    warn :warning_type => 'SQL Injection',
+      :warning_code => :CVE_2014_0080,
+      :message => "Rails #{tracker.config[:rails_version]} contains a SQL injection vulnerability (CVE-2014-0080) with PostgreSQL. Upgrade to 4.0.3",
+      :confidence => CONFIDENCE[:high],
+      :file => gemfile_or_environment,
+      :link_path => "https://groups.google.com/d/msg/rubyonrails-security/Wu96YkTUR6s/pPLBMZrlwvYJ"
+  end
+
   def upgrade_version? versions
     versions.each do |low, high, upgrade|
       return upgrade if version_between? low, high

lib/brakeman/version.rb

 module Brakeman
-  Version = "2.4.0"
+  Version = "2.4.1"
 end

lib/brakeman/warning_codes.rb

@@ -71,7 +71,11 @@ module Brakeman::WarningCodes
     :CVE_2013_6416_call => 68,
     :CVE_2013_6417 => 69,
     :mass_assign_permit! => 70,
-    :ssl_verification_bypass => 71
+    :ssl_verification_bypass => 71,
+    :CVE_2014_0080 => 72,
+    :CVE_2014_0081 => 73,
+    :CVE_2014_0081_call => 74,
+    :CVE_2014_0082 => 75,
   }
 
   def self.code name

test/apps/rails3.2/app/controllers/users_controller.rb

@@ -92,4 +92,8 @@ class UsersController < ApplicationController
   def show_detailed_exceptions?
     false # no warning
   end
+
+  def render_text
+    render :text => "oh noes my service"
+  end
 end

test/apps/rails3/app/controllers/products_controller.rb

@@ -80,4 +80,8 @@ class ProductsController < ApplicationController
       format.xml  { head :ok }
     end
   end
+
+  def render_some_text
+    render :text => "jello"
+  end
 end

test/apps/rails4/Gemfile

@@ -3,7 +3,7 @@ source 'https://rubygems.org'
 # Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
 gem 'rails', '4.0.0'
 
-gem 'sqlite3'
+gem 'pg'
 
 # Gems used only for assets and not required
 # in production environments by default.

test/apps/rails4/app/views/users/index.html.erb

@@ -8,4 +8,6 @@
 
 <%= number_to_currency(params[:cost], params[:currency]) %>
 
-<%= number_to_currency(params[:cost], h(params[:currency])) %> Should not warn
+<%= number_to_human(params[:cost], format: h(params[:format])) %> Should not warn
+
+<%= number_to_percentage(params[:cost], negative_format: params[:format]) %>

test/tests/only_files_option.rb

@@ -74,13 +74,13 @@ class OnlyFilesOptionTests < Test::Unit::TestCase
       :user_input => nil
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "813b00b5c58567fb3f32051578b839cb25fc2d827834a30d4b213a4c126202a2",
+      :warning_code => 73,
+      :fingerprint => "f6981b9c24727ef45040450a1f4b158ae3bc31b4b0343efe853fe12c64881695",
       :warning_type => "Cross Site Scripting",
       :line => nil,
-      :message => /^Rails\ 3\.2\.9\.rc2 has\ a\ vulnerability\ in\ numbe/,
+      :message => /^Rails\ 3\.2\.9\.rc2\ has\ a\ vulnerability\ in\ n/,
       :confidence => 1,
       :relative_path => "Gemfile",
       :user_input => nil

test/tests/rails2.rb

@@ -991,14 +991,16 @@ class Rails2Tests < Test::Unit::TestCase
       :relative_path => "config/environment.rb"
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "1822c8179beeb0358b71c545bad0dd824104aed8b995fe0781c1b6e324417a91",
+      :warning_code => 73,
+      :fingerprint => "dd82650c29c3ec7b77437c32d394641744208b42b2aeb673d54e5f42c51e6c33",
       :warning_type => "Cross Site Scripting",
+      :line => nil,
       :message => /^Rails\ 2\.3\.11\ has\ a\ vulnerability\ in\ numb/,
       :confidence => 1,
-      :relative_path => "config/environment.rb"
+      :relative_path => "config/environment.rb",
+      :user_input => nil
   end
 
   def test_sql_injection_CVE_2013_6417

test/tests/rails3.rb

@@ -16,7 +16,7 @@ class Rails3Tests < Test::Unit::TestCase
       :controller => 1,
       :model => 8,
       :template => 38,
-      :generic => 70
+      :generic => 71
     }
 
     if RUBY_PLATFORM == 'java'
@@ -1149,10 +1149,10 @@ class Rails3Tests < Test::Unit::TestCase
       :relative_path => "Gemfile"
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "813b00b5c58567fb3f32051578b839cb25fc2d827834a30d4b213a4c126202a2",
+      :warning_code => 73,
+      :fingerprint => "f6981b9c24727ef45040450a1f4b158ae3bc31b4b0343efe853fe12c64881695",
       :warning_type => "Cross Site Scripting",
       :line => nil,
       :message => /^Rails\ 3\.0\.3\ has\ a\ vulnerability\ in\ numbe/,
@@ -1173,6 +1173,18 @@ class Rails3Tests < Test::Unit::TestCase
       :user_input => nil
   end
 
+  def test_denial_of_service_CVE_2014_0082
+    assert_warning :type => :warning,
+      :warning_code => 75,
+      :fingerprint => "403a72d08a90043384fe56d3a6bc3e255b8799b380693914143d403607433db7",
+      :warning_type => "Denial of Service",
+      :line => nil,
+      :message => /^Rails\ 3\.0\.3\ has\ a\ denial\ of\ service\ vuln/,
+      :confidence => 0,
+      :relative_path => "Gemfile",
+      :user_input => nil
+  end
+
   def test_http_only_session_setting
     assert_warning :type => :warning,
       :warning_type => "Session Setting",

test/tests/rails31.rb

@@ -820,10 +820,10 @@ class Rails31Tests < Test::Unit::TestCase
       :relative_path => "Gemfile"
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "813b00b5c58567fb3f32051578b839cb25fc2d827834a30d4b213a4c126202a2",
+      :warning_code => 73,
+      :fingerprint => "f6981b9c24727ef45040450a1f4b158ae3bc31b4b0343efe853fe12c64881695",
       :warning_type => "Cross Site Scripting",
       :line => nil,
       :message => /^Rails\ 3\.1\.0\ has\ a\ vulnerability\ in\ numbe/,

test/tests/rails32.rb

@@ -11,7 +11,7 @@ class Rails32Tests < Test::Unit::TestCase
       :controller => 0,
       :model => 5,
       :template => 11,
-      :generic => 10 }
+      :generic => 11 }
 
     if RUBY_PLATFORM == 'java'
       @expected[:generic] += 1
@@ -99,13 +99,13 @@ class Rails32Tests < Test::Unit::TestCase
       :relative_path => "Gemfile"
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "813b00b5c58567fb3f32051578b839cb25fc2d827834a30d4b213a4c126202a2",
+      :warning_code => 73,
+      :fingerprint => "f6981b9c24727ef45040450a1f4b158ae3bc31b4b0343efe853fe12c64881695",
       :warning_type => "Cross Site Scripting",
       :line => nil,
-      :message => /^Rails\ 3\.2\.9\.rc2 has\ a\ vulnerability\ in\ numbe/,
+      :message => /^Rails\ 3\.2\.9\.rc2\ has\ a\ vulnerability\ in\ n/,
       :confidence => 1,
       :relative_path => "Gemfile",
       :user_input => nil
@@ -123,6 +123,18 @@ class Rails32Tests < Test::Unit::TestCase
       :user_input => nil
   end
 
+  def test_denial_of_service_CVE_2014_0082
+    assert_warning :type => :warning,
+      :warning_code => 75,
+      :fingerprint => "403a72d08a90043384fe56d3a6bc3e255b8799b380693914143d403607433db7",
+      :warning_type => "Denial of Service",
+      :line => nil,
+      :message => /^Rails\ 3\.2\.9\.rc2\ has\ a\ denial\ of\ service\ /,
+      :confidence => 0,
+      :relative_path => "Gemfile",
+      :user_input => nil
+  end
+
   def test_redirect_1
     assert_warning :type => :warning,
       :warning_type => "Redirect",

test/tests/rails4.rb

@@ -14,8 +14,8 @@ class Rails4Tests < Test::Unit::TestCase
     @expected ||= {
       :controller => 0,
       :model => 1,
-      :template => 1,
-      :generic => 18
+      :template => 2,
+      :generic => 19
     }
   end
 
@@ -224,16 +224,26 @@ class Rails4Tests < Test::Unit::TestCase
       :relative_path => "Gemfile"
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :template,
-      :warning_code => 66,
-      :fingerprint => "0fb96b5f4b3a4dcdc677d126f492441e2f7b46880563a977b1246b30d3c117a0",
+      :warning_code => 74,
+      :fingerprint => "2d06291f03b443619407093e5921ee1e4eb77b1bf045607d776d9493da4a3f95",
       :warning_type => "Cross Site Scripting",
       :line => 9,
-      :message => /^Currency\ value\ in\ number_to_currency\ is\ /,
+      :message => /^Format\ options\ in\ number_to_currency\ are/,
       :confidence => 0,
       :relative_path => "app/views/users/index.html.erb",
       :user_input => s(:call, s(:call, nil, :params), :[], s(:lit, :currency))
+
+    assert_warning :type => :template,
+      :warning_code => 74,
+      :fingerprint => "c5f481595217e42fbeaf40f32e6407e66d64d246a9729c2c199053e64365ac96",
+      :warning_type => "Cross Site Scripting",
+      :line => 12,
+      :message => /^Format\ options\ in\ number_to_percentage\ a/,
+      :confidence => 0,
+      :relative_path => "app/views/users/index.html.erb",
+      :user_input => s(:call, s(:call, nil, :params), :[], s(:lit, :format))
   end
 
   def test_simple_format_xss_CVE_2013_6416
@@ -260,6 +270,18 @@ class Rails4Tests < Test::Unit::TestCase
       :user_input => nil
   end
 
+  def test_sql_injection_CVE_2014_0080
+    assert_warning :type => :warning,
+      :warning_code => 72,
+      :fingerprint => "0ba20216bdda1cc067f9e4795bdb0d9224fd23c58317ecc09db67b6b38a2d0f0",
+      :warning_type => "SQL Injection",
+      :line => nil,
+      :message => /^Rails\ 4\.0\.0\ contains\ a\ SQL\ injection\ vul/,
+      :confidence => 0,
+      :relative_path => "Gemfile",
+      :user_input => nil
+  end
+
   def test_mass_assignment_with_permit!
     assert_warning :type => :warning,
       :warning_code => 70,

test/tests/rails4_with_engines.rb

@@ -28,10 +28,10 @@ class Rails4WithEnginesTests < Test::Unit::TestCase
       :relative_path => "Gemfile"
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "813b00b5c58567fb3f32051578b839cb25fc2d827834a30d4b213a4c126202a2",
+      :warning_code => 73,
+      :fingerprint => "f6981b9c24727ef45040450a1f4b158ae3bc31b4b0343efe853fe12c64881695",
       :warning_type => "Cross Site Scripting",
       :line => nil,
       :message => /^Rails\ 4\.0\.0\ has\ a\ vulnerability\ in\ numbe/,

test/tests/rails_with_xss_plugin.rb

@@ -381,10 +381,10 @@ class RailsWithXssPluginTests < Test::Unit::TestCase
       :user_input => nil
   end
 
-  def test_number_to_currency_CVE_2013_6415
+  def test_number_to_currency_CVE_2014_0081
     assert_warning :type => :warning,
-      :warning_code => 65,
-      :fingerprint => "813b00b5c58567fb3f32051578b839cb25fc2d827834a30d4b213a4c126202a2",
+      :warning_code => 73,
+      :fingerprint => "f6981b9c24727ef45040450a1f4b158ae3bc31b4b0343efe853fe12c64881695",
       :warning_type => "Cross Site Scripting",
       :line => nil,
       :message => /^Rails\ 2\.3\.14\ has\ a\ vulnerability\ in\ numb/,

test/tests/rescanner.rb

@@ -265,6 +265,25 @@ class RescannerTests < Test::Unit::TestCase
     assert_reindex :none
     assert_changes
     assert_new 0
-    assert_fixed 3
+    assert_fixed 2
+  end
+
+  def test_gemfile_rails_version_fix_CVE_2014_0082
+    gemfile = "Gemfile.lock"
+
+    before_rescan_of gemfile do
+      replace gemfile, "rails (3.2.9.rc2)", "rails (3.2.17)"
+    end
+
+    #@original is actually modified
+    assert @original.config[:rails_version], "3.2.17"
+    assert_reindex :none
+    assert_changes
+    assert_new 0
+    if RUBY_PLATFORM == "java"
+      assert_fixed 10
+    else
+      assert_fixed 9
+    end
   end
 end