Merge pull request #1192 from presidentbeef/warn_on_use_of_sanitize_i…

…n_link_to

Warn about dangerous link_to href with sanitize()

lib/brakeman/checks/check_link_to_href.rb

@@ -71,14 +71,15 @@ def process_result result
     end
   end
 
+  CHECK_INSIDE_METHODS = [:url_for, :h, :sanitize]
+
   def check_argument? url_arg
     return unless call? url_arg
 
     target = url_arg.target
     method = url_arg.method
 
-    method == :url_for or
-      method == :h or
+    CHECK_INSIDE_METHODS.include? method or
       cgi_escaped? target, method
   end
 

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

@@ -5,3 +5,5 @@
 
 <%= link_to("good", params.merge(:page => 2)) %>
 <%= link_to("xss", url_for(params[:bad])) %>
+
+<%= link_to(image_tag("icons/twitter-gray.svg"), sanitize(@user.home_page), target: "_blank") %>

test/tests/rails5.rb

@@ -12,7 +12,7 @@ def expected
     @@expected ||= {
       :controller => 0,
       :model => 0,
-      :template => 9,
+      :template => 10,
       :generic => 19
     }
   end
@@ -679,6 +679,19 @@ def test_link_to_href_safe_interpolation
       :user_input => s(:call, s(:params), :[], s(:lit, :x))
   end
 
+  def test_cross_site_scripting_sanitize_in_link_to
+    assert_warning :type => :template,
+      :warning_code => 4,
+      :fingerprint => "2247b0928591e951ddb428e97bf4174a36080a196a2f6d6fedd2d7c4428db2a9",
+      :warning_type => "Cross-Site Scripting",
+      :line => 9,
+      :message => /^Potentially\ unsafe\ model\ attribute\ in\ li/,
+      :confidence => 2,
+      :relative_path => "app/views/users/show.html.erb",
+      :code => s(:call, nil, :link_to, s(:call, nil, :image_tag, s(:str, "icons/twitter-gray.svg")), s(:call, nil, :sanitize, s(:call, s(:call, s(:const, :User), :new, s(:call, nil, :user_params)), :home_page)), s(:hash, s(:lit, :target), s(:str, "_blank"))),
+      :user_input => s(:call, s(:call, s(:const, :User), :new, s(:call, nil, :user_params)), :home_page)
+  end
+
   def test_mixed_in_csrf_protection
     assert_no_warning :type => :controller,
       :warning_type => "Cross-Site Request Forgery",