diff --git a/lib/brakeman/checks/check_reverse_tabnabbing.rb b/lib/brakeman/checks/check_reverse_tabnabbing.rb index 6af8f48380..942395c220 100644 --- a/lib/brakeman/checks/check_reverse_tabnabbing.rb +++ b/lib/brakeman/checks/check_reverse_tabnabbing.rb @@ -19,7 +19,11 @@ def process_result result return unless hash? html_opts target = hash_access html_opts, :target - return unless target && string?(target) && target.value == "_blank" + unless target && + (string?(target) && target.value == "_blank" || + symbol?(target) && target.value == :_blank) + return + end target_url = result[:block] ? result[:call].first_arg : result[:call].second_arg diff --git a/test/apps/rails5/app/views/users/show.html.erb b/test/apps/rails5/app/views/users/show.html.erb index 30857deed0..8e0eb0e55b 100644 --- a/test/apps/rails5/app/views/users/show.html.erb +++ b/test/apps/rails5/app/views/users/show.html.erb @@ -10,9 +10,13 @@ <%= link_to '', 'something_static', target: '_blank' %> No warning <%= link_to '', some_url, target: '_blank' %> Warn +<%= link_to '', some_url, target: :_blank %> Warn <%= link_to some_url, target: '_blank' do -%> Warn <% end %> +<%= link_to some_url, target: :_blank do -%> + Warn +<% end %> <%= link_to '', some_url, target: '_blank', rel: 'noopener' %> Weak warning <%= link_to '', some_url, target: '_blank', rel: 'noreferrer' %> Weak warning <%= link_to '', some_url, target: '_blank', rel: 'noopener noreferrer' %> No warning diff --git a/test/tests/rails5.rb b/test/tests/rails5.rb index 17b4e17d97..ecb004e888 100644 --- a/test/tests/rails5.rb +++ b/test/tests/rails5.rb @@ -12,7 +12,7 @@ def expected @@expected ||= { :controller => 0, :model => 0, - :template => 17, + :template => 19, :generic => 21 } end @@ -762,7 +762,6 @@ def test_unscoped_find end def test_reverse_tabnabbing - assert_warning :type => :template, :warning_code => 111, :fingerprint => "a72829f1e36e4d7c4fd71a1b9e39b011137dc3b317a17df2fc7795e08b37cf75", @@ -794,20 +793,42 @@ def test_reverse_tabnabbing assert_warning :type => :template, :warning_code => 111, - :fingerprint => "d81b4e129ad9a43a905c335deb5ca98ef62ce9509cd29d536fcff70c2431dccf", + :fingerprint => "83d6722a5dca630fc2a8ba0bee5b457b3fd9399fabb4575df63ce25b280f5a19", :warning_type => "Reverse Tabnabbing", :line => 13, :message => /^When\ opening\ a\ link\ in\ a\ new\ tab\ without\ setting\ `rel:\ "noopener\ noreferrer"`/, :confidence => 1, :relative_path => "app/views/users/show.html.erb", + :code => s(:call, nil, :link_to, s(:str, ""), s(:call, nil, :some_url), s(:hash, s(:lit, :target), s(:lit, :_blank))), + :user_input => nil + + assert_warning :type => :template, + :warning_code => 111, + :fingerprint => "d81b4e129ad9a43a905c335deb5ca98ef62ce9509cd29d536fcff70c2431dccf", + :warning_type => "Reverse Tabnabbing", + :line => 14, + :message => /^When\ opening\ a\ link\ in\ a\ new\ tab\ without\ setting\ `rel:\ "noopener\ noreferrer"`/, + :confidence => 1, + :relative_path => "app/views/users/show.html.erb", :code => s(:call, nil, :link_to, s(:call, nil, :some_url), s(:hash, s(:lit, :target), s(:str, "_blank"))), :user_input => nil + assert_warning :type => :template, + :warning_code => 111, + :fingerprint => "89231ee3d208edba36dd6c445d4172216c282887e3d9a7b22ab47767ca033b92", + :warning_type => "Reverse Tabnabbing", + :line => 16, + :message => /^When\ opening\ a\ link\ in\ a\ new\ tab\ without\ setting\ `rel:\ "noopener\ noreferrer"`/, + :confidence => 1, + :relative_path => "app/views/users/show.html.erb", + :code => s(:call, nil, :link_to, s(:call, nil, :some_url), s(:hash, s(:lit, :target), s(:lit, :_blank))), + :user_input => nil + assert_warning :type => :template, :warning_code => 111, :fingerprint => "7678f236bb756de38a16d0aeb753a47db32e44c1371aee64e86f04f7bcd7c067", :warning_type => "Reverse Tabnabbing", - :line => 15, + :line => 18, :message => /^When\ opening\ a\ link\ in\ a\ new\ tab\ without\ setting\ `rel:\ "noopener\ noreferrer"`/, :confidence => 2, :relative_path => "app/views/users/show.html.erb", @@ -818,7 +839,7 @@ def test_reverse_tabnabbing :warning_code => 111, :fingerprint => "2a3657324d7d873ae9fb3667534ee2a4df0f7822ec0b379740828aecc2941d8c", :warning_type => "Reverse Tabnabbing", - :line => 16, + :line => 19, :message => /^When\ opening\ a\ link\ in\ a\ new\ tab\ without\ setting\ `rel:\ "noopener\ noreferrer"`/, :confidence => 2, :relative_path => "app/views/users/show.html.erb", @@ -828,21 +849,21 @@ def test_reverse_tabnabbing assert_no_warning :type => :template, :warning_code => 111, :warning_type => "Reverse Tabnabbing", - :line => 17, + :line => 22, :confidence => 2, :relative_path => "app/views/users/show.html.erb" assert_no_warning :type => :template, :warning_code => 111, :warning_type => "Reverse Tabnabbing", - :line => 19, + :line => 23, :confidence => 2, :relative_path => "app/views/users/show.html.erb" assert_no_warning :type => :template, :warning_code => 111, :warning_type => "Reverse Tabnabbing", - :line => 20, + :line => 24, :confidence => 1, :relative_path => "app/views/users/show.html.erb" end