diff options
-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 9651f9733f9..08fb9db6c0f 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -137,7 +137,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 @@ -148,9 +148,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 7179185285c..4b6c7c33e5b 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 |