Persistent XSS in Note objects
Security issue: https://dev.gitlab.org/gitlab/gitlabhq/issues/2839
HackerOne report #508184 by nyangawa
on 2019-03-12, assigned to hackerjuan
:
Summary:
Some cache invalidation and project import logic issues enable an attacker to import a project with XSS payloads in places like MR discussions and similar places where a Note object exists.
Description:
There are basically 3 issues causing the XSS here:
All attributes of Note objects are controllable in project.json
, for example note_html
and cached_markdown_version
.
Now I can control the value of note_html
to contain my XSS payload, but the problem is that the value of this field is a CacheMarkdownField
, it's regenerated from the value of note
during new object creation (when note_object.note_html_invalidated?
returns true). The next question is how to trick GitLab that the field does not need to be regenerated.
in app/models/concerns/cache_markdown_field.rb
define_method(invalidation_method) do
changed_fields = changed_attributes.keys
invalidations = changed_fields & [markdown_field.to_s, *INVALIDATED_BY]
invalidations.delete(markdown_field.to_s) if changed_fields.include?("#{markdown_field}_html")
!invalidations.empty? || !cached_html_up_to_date?(markdown_field)
end
There are 2 checks here (also the last 2 issues):
the first one is:
INVALIDATED_BY = %w[author project].freeze
...
invalidations = changed_fields & [markdown_field.to_s, *INVALIDATED_BY]
invalidations.delete(markdown_field.to_s) if changed_fields.include?("#{markdown_field}_html")
note_object.changed_attributes.keys
=> ["note", "noteable_type", "author_id", "created_at", "updated_at", "project_id", "line_code", "position", "original_position", "note_html", "cached_markdown_version", "change_position", "attachment"]
This check is, unfortunately, voided because
- Neither
author
norproject
is in the changed_attributes list, butauthor_id
andproject_id
-
note
is deleted frominvalidations
becausenote_html
is also changed
So invalidations is empty.
and the other one is:
!cached_html_up_to_date?(markdown_field)
It basically checks whether attribute cached_markdown_version
equals to latest_cached_markdown_version
This is really interesting, because I found that latest_cached_markdown_version
is always 917504 in my GitLab instance (also gitlab.com). Looks like local_version
is always 0 for at least Notes in MR.
def latest_cached_markdown_version
@latest_cached_markdown_version ||= (CacheMarkdownField::CACHE_COMMONMARK_VERSION << 16) | local_version
end
def local_version
return local_markdown_version if has_attribute?(:local_markdown_version)
settings = Gitlab::CurrentSettings.current_application_settings
if settings.respond_to?(:local_markdown_version)
settings.local_markdown_version
else
0
end
end
Finally, I could set note_html
to the XSS payload, and cached_markdown_version
to the magic number to avoid my payload being overwritten by GitLab. :P
Steps To Reproduce:
(Add details for how we can reproduce the issue)
- Create an export of a project with at least 1 discussion in at least 1 merge request.
- Modify the project.json, add field
note_html
andcached_markdown_version
"notes": [
{
"id": 1,
"note": "interesting note here",
"note_html": "<img src=\"test\" onerror=\"alert(document.domain)\"></img>html overwritten",
"cached_markdown_version": 917504,
- Import the modified project
- View the only discussion of the imported project.
Supporting Material/References:
Check https://gitlab.com/Nyangawa/xss/merge_requests/1
, you should be able to see a pop-up.
Impact
This is a typical persistent XSS issue and the link I mentioned above is accessible publicly, so all GitLab users are vulnerable theoretically.
Attachments
Warning: Attachments received through HackerOne, please exercise caution!