Merge pull request #1411 from JacobEvelyn/master

Catch reverse tabnabbing with :_blank symbol
presidentbeef · Oct 22, 2019 · da1cd50 · da1cd50
2 parents 4715677 + 3f5d5d5
commit da1cd50
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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