HTML injection issue in snippets via name parameter

HackerOne report #493154 by rz01 on 2019-02-08, assigned to dappelt:

Hello Gitlab Team,

Summary

I found an HTML injection which is caused by the "full name" parameter being reflected as html when you insert a comment to a snippet. The problem is that there's a missing server-side validation in profile "Full Name" input and so if an user insert malicious html chars inside the first_name field it's possible to store the html in your server.

Vulnerability Live Example

[REDACTED]

Just click on "Open" or on my username and instead of being redirected to my profile (as it should be) you will be redirected to a malicious link (not existing)

Affected Parameter

Full Name input

Tested from

Firefox (latest) / Ubuntu 18.04

HTML injection Payload

<h1><a href="https://notmy.link/malicious.exe" class="pwned">HTML</h1>

Video/PoC

[REDACTED]

If clicked this link redirects to the domain i insert in fullname, which doesn't exist but a potential attacker could change it with a phishing domain for example or a malicious website.

Steps to Reproduce

  • Go to the following url > https://gitlab.com/profile

  • Add the HTML Payload inside your "Full Name" input

  • Go to the following url > [REDACTED]

  • Add a comment with random content and you will see the HTML injected in your "Full Name" field

  • Click on your name or your username and you will be redirected to https://notmy.link/malicious.exe

  • It's also possible to assign class and change the css of the a href tag using pre-existing gitlab classes from local css

Note

I wasn't able to trigger an alert box at the moment but as you should know xss is not limited to an alert box. A potential attacker could add a comment under another user snippet and when that user clicks to see attacker's profile could be redirected to a malicious/phishing url. Also a normal user won't expect a malicious redirect by just pressing on another user profile name.

Suggested Fix

Add a stronger server-side validation on Full Name input in profile area or add a stronger HTML filter in snippets user full name".

Best Regards,
Roberto.

Impact

Exploiting this issue a potential attacker could steal other users' cookies or make other users execute malicious javascript code into their account. In addition to that a potential attacker could add a comment under another user snippet and when that user clicks to see attacker's profile could be redirected to a malicious/phishing url. Also a normal user won't expect a malicious redirect by just pressing on another user profile name.

Attachments

Warning: Attachments received through HackerOne, please exercise caution! *[REDACTED]

Testing Activity

@fjsanpedro

  • Test that the user name is rendering the html name but as text and not as html

Security Testing Activities

  • Test that the user name is rendering the html name but as text and not as html

MR Breakdown

  • In https://gitlab.com/gitlab-org/gitlab/blob/master/app/views/shared/notes/_note.html.haml#L35 we call the method sanitize to clean the user name. Nevertheless, that method ends up calling html_safe. Therefore, if the a attribute is allowed, the user name will be rendered as html.

  • We can easily fix by replacing the method sanitize with simple_sanitize. Nevertheless, this method removes all html tags from the name. To be compliant with other parts of the platform, we shouldn't do this. In other features, we just render the user name without removing any data, just showing it as text instead of as HTML.

  • The fix will pass by removing the call to sanitize in that line, and just show the user name directly as text.

  • 1 MR to fix this and add the spec

Edited by Costel Maxim