diff options
author | Robert Speicher <robert@gitlab.com> | 2017-09-06 15:49:35 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2017-09-06 15:49:35 +0000 |
commit | 821448ecb40986f4c91b504647feb9d3f9303e26 (patch) | |
tree | 986272408168e7e2aa87a27e2f6072431ad1f95d | |
parent | 04a647f96968dbde493a68e31523f48c7265ad7b (diff) | |
parent | 5d23f71604a1154cc2f9bfcecb6df7bd4e198640 (diff) | |
download | gitlab-ce-821448ecb40986f4c91b504647feb9d3f9303e26.tar.gz |
Merge branch 'rs-commit-block-xss-9-3' into 'security-9-3'
[9.3] Prevent a persistent XSS in the commit author block
See merge request gitlab/gitlabhq!2188
-rw-r--r-- | app/helpers/commits_helper.rb | 6 | ||||
-rw-r--r-- | changelogs/unreleased/fix-escape-commit-block.yml | 5 | ||||
-rw-r--r-- | spec/helpers/commits_helper_spec.rb | 22 |
3 files changed, 30 insertions, 3 deletions
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 5b5cdebe919..0d6a947e548 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -134,7 +134,7 @@ module CommitsHelper text = if options[:avatar] - %Q{<span class="commit-#{options[:source]}-name">#{person_name}</span>} + content_tag(:span, person_name, class: "commit-#{options[:source]}-name") else person_name end @@ -145,9 +145,9 @@ module CommitsHelper } if user.nil? - mail_to(source_email, text.html_safe, options) + mail_to(source_email, text, options) else - link_to(text.html_safe, user_path(user), options) + link_to(text, user_path(user), options) end end diff --git a/changelogs/unreleased/fix-escape-commit-block.yml b/changelogs/unreleased/fix-escape-commit-block.yml new file mode 100644 index 00000000000..0665c8e5db2 --- /dev/null +++ b/changelogs/unreleased/fix-escape-commit-block.yml @@ -0,0 +1,5 @@ +--- +title: Prevent a persistent XSS in the commit author block +merge_request: +author: +type: security diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index a2c008790f9..64ed4559dab 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -12,6 +12,17 @@ describe CommitsHelper do expect(helper.commit_author_link(commit)). not_to include('onmouseover="alert(1)"') end + + it 'escapes the author name' do + user = build_stubbed(:user, name: 'Foo <script>alert("XSS")</script>') + + commit = double(author: user, author_name: '', author_email: '') + + expect(helper.commit_author_link(commit)) + .to include('Foo <script>') + expect(helper.commit_author_link(commit, avatar: true)) + .to include('commit-author-name', 'Foo <script>') + end end describe 'commit_committer_link' do @@ -25,6 +36,17 @@ describe CommitsHelper do expect(helper.commit_committer_link(commit)). not_to include('onmouseover="alert(1)"') end + + it 'escapes the commiter name' do + user = build_stubbed(:user, name: 'Foo <script>alert("XSS")</script>') + + commit = double(committer: user, committer_name: '', committer_email: '') + + expect(helper.commit_committer_link(commit)) + .to include('Foo <script>') + expect(helper.commit_committer_link(commit, avatar: true)) + .to include('commit-committer-name', 'Foo <script>') + end end describe '#view_on_environment_button' do |