HackerOne report: Reflected Cross-Site Scripting in go-get Handler - Limited Exploitability
I've verified this finding. It's effectively limited to being an open redirect vulnerability, but a clever attack might be able to do more.
Title: Reflected Cross-Site Scripting in go-get Handler - Limited Exploitability Weakness: Cross-site Scripting (XSS) - Reflected Severity: Low Link: https://hackerone.com/reports/224188 Date: 2017-04-26 23:54:23 +0000 By: @pruby
Details: Test Conditions
This issue was tested in GitLab Community Edition using a combination of code review (against git commit 6c65b63c, April 20 2017) and testing likely issues against a local deployment of Bitnami GitLab Community Edition 9.0.5-0, running on Ubuntu 14.04.5. These are running different versions of GitLab, as we were constrained by time available for deploying systems to test. This issue has not been tested against gitlab.com or other public installations.
Browser testing was conducted in Mozilla Firefox 53.0 (64 bit), Google Chrome 56.0.2924.76 and Internet Explorer 11.
Testing was conducted in research time provided by my employer, Insomnia Security, and was not part of a client engagement.
Issue Description
When the HTTP parameter "go-get" is set to "1", a middleware layer in GitLab is invoked which returns a fixed response, before Rails routing is invoked. At this stage no controller has been loaded, and the path may be set arbitrarily by the requestor.
The path requested is subsequently returned within the response, within an HTML attribute, and without applying HTML encoding. The unsafe inclusion of content within HTML can be observed in the code at:
This results in a limited cross-site scripting vulnerability.
An example invocation which exploits this issue on our local test instance of GitLab is shown:
https://10.37.1.107/=foo'http-equiv='set-cookie'x='?go-get=1
There are a number of constraints on the payload which reduce its impact:
- The path is combined with the GitLab installation's base URI. It must therefore be a valid relative or absolute URI.
- Certain characters will be URI encoded. This includes the HTML angle brackets ('<', '>'), preventing an attacker from injecting arbitrary HTML tags.
- Spaces are URI-encoded. Attributes without spaces between them are a breach of the HTML standard, however were found to be accepted by some common browsers.
- Internet explorer 11 rejects the invalid HTML tags.
- Google Chrome accepts the HTML, but its XSS filter prevents execution.
- Mozilla Firefox accepts the meta tag without spaces.
Impact
Due to constraints on the attacker-provided content, the exploitability of this issue is likely to be limited. The confirmed impact of this issue is equivalent to an open redirect vulnerability, aiding social engineering attacks against GitLab users.
The open redirect issue provides a useful tool for conducting social engineering attacks against users of GitLab. An attacker may distribute a URL to the GitLab installation like the following:
https://10.37.1.107/http:;url=http://www.example.com'http-equiv='refresh'x='?go-get=1
The user may trust this link, as it goes to a site the user presumably knows and trusts. On visiting this link, a victim would be redirected to the target URL - in this case http://www.example.com , which may contain malicious content. In this way, an attacker can exploit the user's trust in the application to steal credentials or damage the reputation of GitLab.
There are a couple of points to be aware of if altering this payload:
-
Note that some characters, such as colons and solidus (forward slash) characters cause errors if included directly in the path. As we are inside an HTML attribute context however, and as neither the ampersand (&) nor semicolon (;) triggers an error, we can use HTML entities to avoid the use of these characters directly.
-
Our chosen path begins with a protocol (http:) as this prevents the GitLab base URI being prepended to the path when joining the URIs. Without this an IP address was prepended in our installation (10.37.1.107) which resulted in a 10-second delay before redirecting. As this may limit the efficacy of this attack, this work-around was introduced. This is likely unnecessary where GitLab CE is hosted under a domain name.
Recommendations
- Whitelist input from untrusted sources against a list of acceptable values or patterns wherever feasible.
- Encode strings appropriately on output within structured languages such as HTML to ensure their interpretation as plain text. Be aware that contexts may be nested - an inclusion might be a URI within HTML, and require encoding of special characters for both of those protocols.
- Update this middleware to use double quotes to delimit HTML attributes. While accepted by major browsers, this is not standardised behaviour. HTML encoding routines are not required to encode single quotes, and this may cause regressions in future.
- Ensure that middleware components abide by the same security standards as the general GitLab application.
- Consider migrating the affected functionality in to a Rails controller. Low-level code outside the framework cannot benefit from protections provided by default.
Timeline: 2017-04-27 00:14:55 +0000: @DouweM (comment [team-only]) Quick thoughts:
Whitelist input from untrusted sources against a list of acceptable values or patterns wherever feasible.
Acceptable pattern would be Gitlab::Regex::FULL_NAMESPACE_REGEX_STR
.
Encode strings appropriately on output within structured languages such as HTML to ensure their interpretation as plain text. Be aware that contexts may be nested - an inclusion might be a URI within HTML, and require encoding of special characters for both of those protocols.
We could use some view helpers instead of directly returning HTML.
Update this middleware to use double quotes to delimit HTML attributes. While accepted by major browsers, this is not standardised behaviour. HTML encoding routines are not required to encode single quotes, and this may cause regressions in future.
Good idea.
Ensure that middleware components abide by the same security standards as the general GitLab application.
They do, this bug could just as easily have been in the Rails portion of the app
Consider migrating the affected functionality in to a Rails controller. Low-level code outside the framework cannot benefit from protections provided by default.
We could do this using a controller, but it would need to circumvent all authentication and actual project existence check logic, and only happen if ?go-get=1
is in the URL.