Merge branch 'strip_tags_CVE_2012_3465'

Add check for CVE-2012-3465

Conflicts:
	test/tests/test_rails3.rb

lib/brakeman/checks/check_cross_site_scripting.rb

@@ -54,6 +54,10 @@ class Brakeman::CheckCrossSiteScripting < Brakeman::BaseCheck
       @ignore_methods << :auto_link
     end
 
+    if version_between? "2.0.0", "2.3.14"
+      @known_dangerous << :strip_tags
+    end
+
     if tracker.options[:rails3]
       @ignore_methods << :select
     end

lib/brakeman/checks/check_strip_tags.rb

 require 'brakeman/checks/base_check'
 
-#Checks for uses of strip_tags in Rails versions before 2.3.13 and 3.0.10
+#Check for uses of strip_tags in Rails versions before 3.0.17, 3.1.8, 3.2.8 (including 2.3.x):
+#https://groups.google.com/d/topic/rubyonrails-security/FgVEtBajcTY/discussion
+#
+#Check for uses of strip_tags in Rails versions before 2.3.13 and 3.0.10:
 #http://groups.google.com/group/rubyonrails-security/browse_thread/thread/2b9130749b74ea12
 class Brakeman::CheckStripTags < Brakeman::BaseCheck
   Brakeman::Checks.add self
 
-  @description = "Report strip_tags vulnerability in versions before 2.3.13 and 3.0.10"
+  @description = "Report strip_tags vulnerabilities CVE-2011-2931 and CVE-2012-3465"
 
   def run_check
-    if (version_between?('2.0.0', '2.3.12') or 
-        version_between?('3.0.0', '3.0.9')) and uses_strip_tags?
+    if uses_strip_tags?
+      cve_2011_2931
+      cve_2012_3465
+    end
+  end
 
+  def cve_2011_2931
+    if version_between?('2.0.0', '2.3.12') or version_between?('3.0.0', '3.0.9')
       if tracker.config[:rails_version] =~ /^3/
-        message = "Versions before 3.0.10 have a vulnerability in strip_tags: CVE-2011-2931"
+        message = "Versions before 3.0.10 have a vulnerability in strip_tags (CVE-2011-2931)"
       else
-        message = "Versions before 2.3.13 have a vulnerability in strip_tags: CVE-2011-2931"
+        message = "Versions before 2.3.13 have a vulnerability in strip_tags (CVE-2011-2931)"
       end
 
       warn :warning_type => "Cross Site Scripting",
         :message => message,
-        :confidence => CONFIDENCE[:high],
         :file => gemfile_or_environment,
+        :confidence => CONFIDENCE[:high],
         :link_path => "https://groups.google.com/d/topic/rubyonrails-security/K5EwdJt06hI/discussion"
     end
   end
 
+  def cve_2012_3465
+    case
+    when (version_between?('2.0.0', '2.3.14') and tracker.config[:escape_html])
+      message = "All Rails 2.x versions have a vulnerability in strip_tags (CVE-2012-3465)"
+    when version_between?('3.0.10', '3.0.16')
+      message = "Rails #{tracker.config[:rails_version]} has a vulnerability in strip_tags (CVE-2012-3465). Upgrade to 3.0.17"
+    when version_between?('3.1.0', '3.1.7')
+      message = "Rails #{tracker.config[:rails_version]} has a vulnerability in strip_tags (CVE-2012-3465). Upgrade to 3.1.8"
+    when version_between?('3.2.0', '3.2.7')
+      message = "Rails #{tracker.config[:rails_version]} has a vulnerability in strip_tags (CVE-2012-3465). Upgrade to 3.2.8"
+    else
+      return
+    end
+
+    warn :warning_type => "Cross Site Scripting",
+      :message => message,
+      :confidence => CONFIDENCE[:high],
+      :file => gemfile_or_environment,
+      :link_path => "https://groups.google.com/d/topic/rubyonrails-security/FgVEtBajcTY/discussion"
+  end
+
   def uses_strip_tags?
     Brakeman.debug "Finding calls to strip_tags()"
 

test/apps/rails2/app/views/home/test_strip_tags.html.erb

+<%= h strip_tags(params[:name]) %>
+
+<%= strip_tags(params[:body]) %>

test/apps/rails3.1/app/views/other/test_strip_tags.html.erb

+<%= strip_tags params[:body] %>

test/apps/rails3/app/views/other/test_strip_tags.html.erb

+<%= strip_tags params[:body] %>

test/apps/rails_with_xss_plugin/app/views/users/show.html.erb

@@ -16,6 +16,8 @@
   <b>Bad:</b> <%= @evil_input %>
 </p>
 
+<%= strip_tags @user.profile %>
+
 <% if @current_user and (@current_user.id == @user.id or @current_user.admin?) %>
 <%= link_to 'Edit', edit_user_path(@user) %> |
 <% end %>

test/tests/test_rails2.rb

@@ -11,14 +11,14 @@ class Rails2Tests < Test::Unit::TestCase
       @expected ||= {
         :controller => 1,
         :model => 2,
-        :template => 31,
-        :warning => 29 }
+        :template => 32,
+        :warning => 30 }
     else
       @expected ||= {
         :controller => 1,
         :model => 2,
-        :template => 31,
-        :warning => 30 }
+        :template => 32,
+        :warning => 31 }
     end
   end
 
@@ -633,6 +633,15 @@ class Rails2Tests < Test::Unit::TestCase
       :file => /test_xss_with_or\.html\.erb/
   end
 
+  def test_cross_site_scripting_strip_tags
+    assert_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 3,
+      :message => /^Unescaped\ parameter\ value/,
+      :confidence => 0,
+      :file => /test_strip_tags\.html\.erb/
+  end
+
   def test_check_send
     assert_warning :type => :warning,
       :warning_type => "Dangerous Send",
@@ -648,5 +657,22 @@ class Rails2Tests < Test::Unit::TestCase
       :confidence => 1,
       :file => /home_controller\.rb/
   end
+
+  def test_strip_tags_CVE_2011_2931
+    assert_warning :type => :warning,
+      :warning_type => "Cross Site Scripting",
+      :message => /^Versions\ before\ 2\.3\.13\ have\ a\ vulnerabil/,
+      :confidence => 0,
+      :file => /environment\.rb/
+  end
+
+  def test_strip_tags_CVE_2012_3465_high
+    assert_warning :type => :template,
+      :warning_type => "Cross Site Scripting",
+      :line => 3,
+      :message => /^Unescaped\ parameter\ value/,
+      :confidence => 0,
+      :file => /test_strip_tags\.html\.erb/
+  end
 end
 

test/tests/test_rails3.rb

@@ -15,7 +15,7 @@ class Rails3Tests < Test::Unit::TestCase
       :controller => 1,
       :model => 5,
       :template => 23,
-      :warning => 27
+      :warning => 28
     }
   end
 
@@ -611,4 +611,12 @@ class Rails3Tests < Test::Unit::TestCase
       :confidence => 0,
       :file => /Gemfile/
   end
+
+  def test_strip_tags_CVE_2012_3465
+    assert_warning :type => :warning,
+      :warning_type => "Cross Site Scripting",
+      :message => /^Versions\ before\ 3\.0\.10\ have\ a\ vulnerabil/,
+      :confidence => 0,
+      :file => /Gemfile/
+  end
 end

test/tests/test_rails31.rb

@@ -15,7 +15,7 @@ class Rails31Tests < Test::Unit::TestCase
       :model => 0,
       :template => 12,
       :controller => 1,
-      :warning => 46 }
+      :warning => 47 }
   end
 
   def test_without_protection
@@ -579,4 +579,12 @@ class Rails31Tests < Test::Unit::TestCase
       :confidence => 2,
       :file => /Gemfile/
   end
+
+  def test_strip_tags_CVE_2012_3465
+    assert_warning :type => :warning,
+      :warning_type => "Cross Site Scripting",
+      :message => /^Rails\ 3\.1\.0\ has\ a\ vulnerability\ in\ strip/,
+      :confidence => 0,
+      :file => /Gemfile/
+  end
 end

test/tests/test_rails_with_xss_plugin.rb

@@ -11,7 +11,7 @@ class RailsWithXssPluginTests < Test::Unit::TestCase
       :controller => 1,
       :model => 3,
       :template => 1,
-      :warning => 13 }
+      :warning => 14 }
   end
 
   def report
@@ -243,4 +243,11 @@ class RailsWithXssPluginTests < Test::Unit::TestCase
       :file => /user\.rb/
   end
 
+  def test_strip_tags_CVE_2012_3465
+    assert_warning :type => :warning,
+      :warning_type => "Cross Site Scripting",
+      :message => /^All\ Rails\ 2\.x\ versions\ have\ a\ vulnerabil/,
+      :confidence => 0,
+      :file => /Gemfile/
+  end
 end