Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch reverse tabnabbing with :_blank symbol #1411

Merged
merged 1 commit into from Oct 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/brakeman/checks/check_reverse_tabnabbing.rb
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions test/apps/rails5/app/views/users/show.html.erb
Expand Up @@ -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
Expand Down
37 changes: 29 additions & 8 deletions test/tests/rails5.rb
Expand Up @@ -12,7 +12,7 @@ def expected
@@expected ||= {
:controller => 0,
:model => 0,
:template => 17,
:template => 19,
:generic => 21
}
end
Expand Down Expand Up @@ -762,7 +762,6 @@ def test_unscoped_find
end

def test_reverse_tabnabbing

assert_warning :type => :template,
:warning_code => 111,
:fingerprint => "a72829f1e36e4d7c4fd71a1b9e39b011137dc3b317a17df2fc7795e08b37cf75",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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
Expand Down