Page MenuHomePhabricator
Create Task
Maniphest T270195

Reflected Cross-Site scripting (XSS) vulnerability in analytics-quarry-web
Closed, ResolvedPublicSecurity
Actions

Assigned To
Reedy
Authored By
Reedy
Dec 15 2020, 4:39 PM
Tags
Referenced Files
F33946428: 0001-SECURITY-Set-correct-Content-Type-Mime-Type-on-api-p.patch
Dec 15 2020, 5:06 PM
F33946416: 0001-SECURITY-Set-correct-Content-Type-Mime-Type-on-api-p.patch
Dec 15 2020, 4:57 PM
Subscribers
Aklapper
Antoniomoralesmaldonado
Framawiki
gerritbot
• Marostegui
Reedy
sbassett
Tokens
"Love" token, awarded by ema."Love" token, awarded by jcrespo.

Description

From security@ email

GitHub Security Lab (GHSL) Vulnerability Report: GHSL-2020-336
The GitHub Security Lab team has identified potential security vulnerabilities in wikimedia/analytics-quarry-web.

We are committed to working with you to help resolve these issues. In this report you will find everything you need to effectively coordinate a resolution of these issues with the GHSL team.

If at any point you have concerns or questions about this process, please do not hesitate to reach out to us at securitylab@github.com (please include GHSL-2020-336 as a reference).

If you are NOT the correct point of contact for this report, please let us know!

Summary
A reflected Cross-Site scripting (XSS) vulnerability has been found in analytics-quarry-web

Product
https://github.com/wikimedia/analytics-quarry-web

Tested Version
Latest commit at the time of reporting (December 14, 2020).

Details
The server responds with return Response(json.dumps(...)) without setting proper mime-type (application/json).

This becomes problematic for the preference handling defined here: https://github.com/wikimedia/analytics-quarry-web/blob/085a51b2dee8b58882276d9fe090174252edb85e/quarry/web/app.py#L395-L412

You can exploit this vulnerability by tricking a logged in user to visit vulnerable URL.

PoC:

Visit official Quarry site https://quarry.wmflabs.org/ or follow setup instructions on repo. (I found official site from here)
Log in with a wiki-media acocunt
Visit vulnerable URL: https://quarry.wmflabs.org/api/preferences/get/%3Cimg%20src=0%20onerror=alert(0)%3E
Impact
XSS can cause a variety of problems for the end user that range in severity from an annoyance to complete account compromise. The most severe XSS attacks involve disclosure of the user’s session cookie, allowing an attacker to hijack the user’s session and take over the account.

GitHub Security Advisories
We recommend you create a private GitHub Security Advisory for these findings. This also allows you to invite the GHSL team to collaborate and further discuss these findings in private before they are published.

Credit
This issue was discovered and reported by CodeQL Python team.

Contact
You can contact the GHSL team at securitylab@github.com, please include GHSL-2020-336 in any communication regarding this issue.

Disclosure Policy
This report is subject to our coordinated disclosure policy.

Details

SubjectRepoBranchLines +/-
SECURITY: Set correct Mime Type on /api/preferencesanalytics/quarry/webmaster+12 -3
Customize query in gerrit

Related Objects

Event Timeline

Reedy created this task.Dec 15 2020, 4:39 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 15 2020, 4:39 PM
Reedy added projects: Quarry, Analytics, Vuln-XSS.Dec 15 2020, 4:39 PM
Reedy added a project: cloud-services-team.
Restricted Application edited projects, added cloud-services-team (Kanban); removed cloud-services-team. · View Herald TranscriptDec 15 2020, 4:41 PM
Reedy added a comment.Dec 15 2020, 4:46 PM

Obviously not the same, but it's not too dissimilar to T242355: Trivial Quarry XSS: Tabular data CSV / TSV are not escaped and sent as text/html and the fix at rANQW085a51b2dee8: SECURITY: Update mime and content types downloads...

Reedy added a comment.EditedDec 15 2020, 4:49 PM

Is the fix really as simple as this?

diff --git a/quarry/web/app.py b/quarry/web/app.py
index 13251eb..31c426b 100644
--- a/quarry/web/app.py
+++ b/quarry/web/app.py
@@ -398,9 +398,9 @@ def pref_get(key):
         return "Authentication required", 401
 
     if key in get_preferences():
-        return Response(json.dumps({'key': key, 'value': get_preferences()[key]}))
+        return Response(json.dumps({'key': key, 'value': get_preferences()[key]}),mimetype='application/json',content_type='application/json; charset=utf-8')
     else:
-        return Response(json.dumps({'key': key, 'error': 'novalue'}))
+        return Response(json.dumps({'key': key, 'error': 'novalue'}),mimetype='application/json',content_type='application/json; charset=utf-8')
 
 
 @app.route("/api/preferences/set/<key>/<value>")
@@ -409,7 +409,7 @@ def pref_set(key, value):
         return "Authentication required", 401
 
     get_preferences()[key] = (None if value == 'null' else value)
-    return Response(json.dumps({'key': key, 'success': ''})), 201
+    return Response(json.dumps({'key': key, 'success': ''}),mimetype='application/json',content_type='application/json; charset=utf-8'), 201
 
 
 if __name__ == '__main__':
sbassett subscribed.Dec 15 2020, 4:50 PM
In T270195#6692650, @Reedy wrote:

Is the fix really as simple as this?

Probably :)

Reedy added a comment.Dec 15 2020, 4:57 PM

0001-SECURITY-Set-correct-Content-Type-Mime-Type-on-api-p.patch1 KBDownload

Reedy triaged this task as High priority.Dec 15 2020, 4:59 PM
Reedy added a comment.Dec 15 2020, 5:06 PM

I guess the content type is superfluous (and not present in other code in the file)

commit bb83dad383a5d3c8ce5c21d28b6522c42ebc1668 (HEAD -> master)
Author: Reedy <reedy@wikimedia.org>
Date:   Tue Dec 15 16:55:55 2020 +0000

    SECURITY: Set correct Content-Type/Mime Type on /api/preferences
    
    Prevents a Reflected Cross-Site scripting (XSS) vulnerability
    
    Bug: T270195

diff --git a/quarry/web/app.py b/quarry/web/app.py
index 13251eb..845a60f 100644
--- a/quarry/web/app.py
+++ b/quarry/web/app.py
@@ -398,9 +398,15 @@ def pref_get(key):
         return "Authentication required", 401
 
     if key in get_preferences():
-        return Response(json.dumps({'key': key, 'value': get_preferences()[key]}))
+        return Response(
+            json.dumps({'key': key, 'value': get_preferences()[key]}),
+            mimetype='application/json'
+        )
     else:
-        return Response(json.dumps({'key': key, 'error': 'novalue'}))
+        return Response(
+            json.dumps({'key': key, 'error': 'novalue'}),
+            mimetype='application/json'
+        )
 
 
 @app.route("/api/preferences/set/<key>/<value>")
@@ -409,7 +415,10 @@ def pref_set(key, value):
         return "Authentication required", 401
 
     get_preferences()[key] = (None if value == 'null' else value)
-    return Response(json.dumps({'key': key, 'success': ''})), 201
+    return Response(
+        json.dumps({'key': key, 'success': ''}),
+        mimetype='application/json'
+    ), 201
 
 
 if __name__ == '__main__':

0001-SECURITY-Set-correct-Content-Type-Mime-Type-on-api-p.patch1 KBDownload

Reedy added a comment.Dec 15 2020, 5:06 PM

Code originally added by rANQWbcea4b2105eb: Implement user prefs and browser notifications for T124625: Show desktop notification when a query is done

Reedy added a subscriber: Framawiki.Dec 15 2020, 5:08 PM
jcrespo awarded a token.Dec 15 2020, 5:09 PM
Reedy added a subscriber: gerritbot.Dec 15 2020, 5:31 PM
• Marostegui subscribed.Dec 15 2020, 5:31 PM
Reedy added a comment.Dec 15 2020, 5:31 PM

Patch deployed... XSS seems to be fixed

gerritbot added a comment.Dec 15 2020, 5:37 PM

Change 649711 had a related patch set uploaded (by Reedy; owner: Reedy):
[analytics/quarry/web@master] SECURITY: Set correct Mime Type on /api/preferences

https://gerrit.wikimedia.org/r/649711

gerritbot added a project: Patch-For-Review.Dec 15 2020, 5:37 PM

Change 649711 merged by jenkins-bot:
[analytics/quarry/web@master] SECURITY: Set correct Mime Type on /api/preferences

https://gerrit.wikimedia.org/r/649711

Reedy closed this task as Resolved.Dec 15 2020, 5:44 PM
Reedy claimed this task.

Pulled onto hosts, workers restarted

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Dec 15 2020, 5:44 PM
Reedy changed the edit policy from "Custom Policy" to "All Users".
Reedy added a subscriber: Antoniomoralesmaldonado.Dec 15 2020, 5:50 PM
• ema awarded a token.Dec 16 2020, 12:08 PM
Maintenance_bot removed a project: Patch-For-Review.Dec 16 2020, 12:10 PM
sbassett moved this task from Incoming to Our Part Is Done on the Security-Team board.Dec 21 2020, 4:57 PM
Framawiki added a comment.Jan 23 2021, 3:33 PM

I have just read this ticket, sorry for the code I added and thanks for the remediation.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits