diff --git a/lib/brakeman/checks/check_link_to_href.rb b/lib/brakeman/checks/check_link_to_href.rb index ab1c06513b..aab28f2e2a 100644 --- a/lib/brakeman/checks/check_link_to_href.rb +++ b/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 diff --git a/test/apps/rails5/app/views/users/show.html.erb b/test/apps/rails5/app/views/users/show.html.erb index 1144299726..52ba2d1410 100644 --- a/test/apps/rails5/app/views/users/show.html.erb +++ b/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") %> diff --git a/test/tests/rails5.rb b/test/tests/rails5.rb index 4f661e5b15..4b7734ca09 100644 --- a/test/tests/rails5.rb +++ b/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",