Merge branch 'fix_mail_to_vuln_check'

lib/brakeman/checks/check_mail_to.rb

@@ -23,7 +23,8 @@ class Brakeman::CheckMailTo < Brakeman::BaseCheck
         :warning_type => "Mail Link",
         :message => message,
         :confidence => CONFIDENCE[:high],
-        :file => gemfile_or_environment
+        :file => gemfile_or_environment,
+        :link_path => "https://groups.google.com/d/topic/rubyonrails-security/8CpI7egxX4E/discussion"
     end
   end
 
@@ -33,13 +34,10 @@ class Brakeman::CheckMailTo < Brakeman::BaseCheck
     Brakeman.debug "Checking calls to mail_to for javascript encoding"
 
     tracker.find_call(:target => false, :method => :mail_to).each do |result|
-      call = result[:call]
-      args = call.args
-
-      args.each do |arg|
+      result[:call].arglist.each do |arg|
         if hash? arg
-          if hash_access(arg, :javascript)
-            return result
+          if option = hash_access(arg, :encode)
+            return result if symbol? option and option.value == :javascript
           end
         end
       end

test/apps/rails3/Gemfile

 source 'http://rubygems.org'
 
-gem 'rails', '3.0.5'
+gem 'rails', '3.0.3'
 
 # Bundle edge Rails instead:
 # gem 'rails', :git => 'git://github.com/rails/rails.git'

test/apps/rails3/Gemfile.lock

@@ -2,12 +2,12 @@ GEM
   remote: http://rubygems.org/
   specs:
     abstract (1.0.0)
-    actionmailer (3.0.5)
-      actionpack (= 3.0.5)
+    actionmailer (3.0.3)
+      actionpack (= 3.0.3)
       mail (~> 2.2.15)
-    actionpack (3.0.5)
-      activemodel (= 3.0.5)
-      activesupport (= 3.0.5)
+    actionpack (3.0.3)
+      activemodel (= 3.0.3)
+      activesupport (= 3.0.3)
       builder (~> 2.1.2)
       erubis (~> 2.6.6)
       i18n (~> 0.4)
@@ -15,19 +15,19 @@ GEM
       rack-mount (~> 0.6.13)
       rack-test (~> 0.5.7)
       tzinfo (~> 0.3.23)
-    activemodel (3.0.5)
-      activesupport (= 3.0.5)
+    activemodel (3.0.3)
+      activesupport (= 3.0.3)
       builder (~> 2.1.2)
       i18n (~> 0.4)
-    activerecord (3.0.5)
-      activemodel (= 3.0.5)
-      activesupport (= 3.0.5)
+    activerecord (3.0.3)
+      activemodel (= 3.0.3)
+      activesupport (= 3.0.3)
       arel (~> 2.0.2)
       tzinfo (~> 0.3.23)
-    activeresource (3.0.5)
-      activemodel (= 3.0.5)
-      activesupport (= 3.0.5)
-    activesupport (3.0.5)
+    activeresource (3.0.3)
+      activemodel (= 3.0.3)
+      activesupport (= 3.0.3)
+    activesupport (3.0.3)
     arel (2.0.10)
     builder (2.1.2)
     erubis (2.6.6)
@@ -45,17 +45,17 @@ GEM
       rack (>= 1.0.0)
     rack-test (0.5.7)
       rack (>= 1.0)
-    rails (3.0.5)
-      actionmailer (= 3.0.5)
-      actionpack (= 3.0.5)
-      activerecord (= 3.0.5)
-      activeresource (= 3.0.5)
-      activesupport (= 3.0.5)
+    rails (3.0.3)
+      actionmailer (= 3.0.3)
+      actionpack (= 3.0.3)
+      activerecord (= 3.0.3)
+      activeresource (= 3.0.3)
+      activesupport (= 3.0.3)
       bundler (~> 1.0)
-      railties (= 3.0.5)
-    railties (3.0.5)
-      actionpack (= 3.0.5)
-      activesupport (= 3.0.5)
+      railties (= 3.0.3)
+    railties (3.0.3)
+      actionpack (= 3.0.3)
+      activesupport (= 3.0.3)
       rake (>= 0.8.7)
       thor (~> 0.14.4)
     rake (0.9.2)
@@ -70,5 +70,5 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  rails (= 3.0.5)
+  rails (= 3.0.3)
   sqlite3

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

@@ -39,4 +39,8 @@ class OtherController < ApplicationController
   def test_render_with_nonsymbol_key
     render x => :y
   end
+
+  def test_mail_to
+    @user = User.find(current_user)
+  end
 end

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

+<%= mail_to @user.email, @user.name, :encode => :javascript %>
+
+Should not warn:
+<%= mail_to @user.email, @user.name, :encode => :hex %>

test/apps/rails3/config/routes.rb

@@ -11,6 +11,8 @@ Rails3::Application.routes.draw do
 
   get "other/test_send_file"
 
+  get "other/test_mail_to"
+
   get "home/index"
 
   get "home/test_params"

test/tests/test_rails3.rb

@@ -14,7 +14,7 @@ class Rails3Tests < Test::Unit::TestCase
     @expected ||= {
       :controller => 1,
       :model => 5,
-      :template => 29,
+      :template => 30,
       :warning => 30
     }
   end
@@ -532,7 +532,7 @@ class Rails3Tests < Test::Unit::TestCase
 
   def test_default_routes
     assert_warning :warning_type => "Default Routes",
-      :line => 95,
+      :line => 97,
       :message => /All public methods in controllers are available as actions/,
       :file => /routes\.rb/
   end
@@ -572,7 +572,7 @@ class Rails3Tests < Test::Unit::TestCase
   def test_string_buffer_manipulation_bug
     assert_warning :type => :warning,
       :warning_type => "Cross Site Scripting",
-      :message => /^Rails 3.0.5 has a vulnerabilty in SafeBuffer. Upgrade to 3.0.12/,
+      :message => /^Rails 3\.\d\.\d has a vulnerabilty in SafeBuffer. Upgrade to 3.0.12/,
       :confidence => 1,
       :file => /Gemfile/
   end
@@ -653,7 +653,7 @@ class Rails3Tests < Test::Unit::TestCase
     assert_warning :type => :template,
       :warning_type => "Cross Site Scripting",
       :line => 3,
-      :message => /^Upgrade\ to\ Rails\ 3\.0\.17,\ 3\.0\.5\ select_ta/,
+      :message => /^Upgrade\ to\ Rails\ 3\.0\.17,\ 3\.0\.3\ select_ta/,
       :confidence => 0,
       :file => /test_select_tag\.html\.erb/
   end
@@ -661,7 +661,7 @@ class Rails3Tests < Test::Unit::TestCase
   def test_cross_site_scripting_single_quotes_CVE_2012_3464
     assert_warning :type => :warning,
       :warning_type => "Cross Site Scripting",
-      :message => /^Rails\ 3\.0\.5\ does\ not\ escape\ single\ quote/,
+      :message => /^Rails\ 3\.0\.3\ does\ not\ escape\ single\ quote/,
       :confidence => 1,
       :file => /Gemfile/
   end
@@ -681,4 +681,13 @@ class Rails3Tests < Test::Unit::TestCase
       :confidence => 0,
       :file => /Gemfile/
   end
+
+  def test_mail_link_CVE_2011_0446
+    assert_warning :type => :template,
+      :warning_type => "Mail Link",
+      :line => 1,
+      :message => /^Vulnerability\ in\ mail_to\ using\ javascrip/,
+      :confidence => 0,
+      :file => /Gemfile/
+  end
 end