Page MenuHomePhabricator

XSS in Wikidata Query Service UI (i18n messages) - CVE-2019-19327
Closed, ResolvedPublic

Description

TL;DR: some i18n messages used as HTML; imported nightly from translatewiki, but not automatically deployed to query.wikidata.org

The Wikidata Query Service UI (wikidata/query/gui in Gerrit) uses the “x results in y ms” message directly as HTML:

wikibase/queryService/ui/ResultView.js
$( '#response-summary' ).html(
    wikibase.queryService.ui.i18n.getMessage(
        'wdqs-app-resultbrowser-response-summary',
        '$1 results in $2 ms',
        [ api.getResultLength(), api.getExecutionTime() ]
    )
);

If the message (either the English source or a translation) is edited to contain a <script> tag, the contents will be executed as soon as a query result is shown (which, for embed.html, is as soon as the page loads). To reproduce this issue, clone the repository, run npm install, apply a patch similar to

diff --git a/i18n/en.json b/i18n/en.json
index e588694..f91a1b4 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -67 +67 @@
-    "wdqs-app-resultbrowser-response-summary": "$1 {{PLURAL:$1|result|results}} in $2&nbsp;ms",
+    "wdqs-app-resultbrowser-response-summary": "$1 {{PLURAL:$1|result|results}} in $2&nbsp;ms<script>alert('hi')</script>",

then run npm run start and in the resulting browser window enter any query (e. g. ASK{}) and run it (e. g. pressing Ctrl+Enter, or clicking the blue “play” button).

Translations are imported from translatewiki.net by l10n-bot every night; however, they are not deployed automatically. (There is also a Jenkins job that rejects l10n-bot changes that appear to add HTML, which should prevent the automatic import.) First, a corresponding update to the wikidata/query/gui-deploy repository needs to be built – this is supposed to happen automatically, but is currently broken due to T235651. Then, that update needs to be merged, and then the gui submodule of the wikidata/query/deploy needs to be updated to point to the new build (with another Gerrit change), and this change needs to be deployed (with scap). (This process is subject to change in T235639.) In theory, those reviews can detect the malicious message, but in practice I don’t think we expect actual code review to take place in those stages.

Details

Related Gerrit Patches:
wikidata/query/gui : masterFix two HTML injections

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptThu, Nov 21, 11:51 AM
Restricted Application added a project: Wikidata. · View Herald TranscriptThu, Nov 21, 11:52 AM

Found by running git grep -F '.html(' wikibase/ and skimming through the results. This was the fifth one, out of 25 total. Stay tuned for how many more I find!

Edit: Only one more, T238824.

As far as I can tell, we only use .html instead of .text so that the &nbsp; in the message will be correctly interpreted. There are some ways to decode HTML entities in JavaScript (e. g. here), most of the complete ones boiling down to some form of newElement.innerHtml = ...; return newElement.textContent, but those don’t feel that great to me. And since &nbsp; is the only entity used in all the current translations, we may as well keep it simple:

diff --git a/wikibase/queryService/ui/ResultView.js b/wikibase/queryService/ui/ResultView.js
index 08da446..11f4472 100644
--- a/wikibase/queryService/ui/ResultView.js
+++ b/wikibase/queryService/ui/ResultView.js
@@ -364,12 +364,12 @@ wikibase.queryService.ui.ResultView = ( function( $, download, window ) {
 	SELF.prototype._handleQueryResult = function() {
 		var api = this._sparqlApi;
 
-		$( '#response-summary' ).html(
+		$( '#response-summary' ).text(
 			wikibase.queryService.ui.i18n.getMessage(
 				'wdqs-app-resultbrowser-response-summary',
 				'$1 results in $2&nbsp;ms',
 				[ api.getResultLength(), api.getExecutionTime() ]
-			)
+			).replace( /&nbsp;/g, '\xA0' )
 		);
 		$( '.result' ).show();
Addshore moved this task from incoming to in progress on the Wikidata board.Thu, Nov 21, 1:58 PM

Lets deploy the fix for this one alongside T238824
I'll use this ticket for communication and planning etc so we keep everything in one place.
I just pinged @Lucas_Werkmeister_WMDE to make a patch similar to the recent XSS issue so we can deploy to production and then go through the same sequence as last time for releasing code for everyone else.

Patch for /src/deployment/wdqs/wdqs/gui/ is at /home/lucaswerkmeister-wmde/T238822+T238824.patch on deployment.eqiad.wmnet.

The patch was generated by running grunt security, using the new Grunt task I’m adding in https://gerrit.wikimedia.org/r/552293.

The patch to wikidata/query/gui itself (before build) is at P9720 (the paste should be visible to all subscribers of this task, if I did the custom policy correctly).

sbassett triaged this task as High priority.Thu, Nov 21, 9:38 PM
sbassett moved this task from Backlog / Other to External (Non-WMF) Issues on the Security board.
sbassett added a project: Security-Team.
sbassett moved this task from Incoming to Watching on the Security-Team board.

@Lucas_Werkmeister_WMDE - not sure if you're aware, but there is a bit of automated mitigation running in CI to deal with these kinds of i18n scenarios, see:

  1. jjb: https://integration.wikimedia.org/ci/job/mediawiki-i18n-check-docker/
  2. examples: https://gerrit.wikimedia.org/r/q/owner:l10n-bot%2540translatewiki.net+html

Currently jenkins-bot will -1 any patch where mediawiki-i18n-check-docker fails, which triggers a manual review by some twn-affiliated developers. You can see the script (really just a grep) being run towards the end of the console output within some recent failed builds like this one. The script and this process aren't great, but we needed a stop-gap measure due to some attacks against twn over the last couple of years.

Ah, thanks, that’s good to know.

Lucas_Werkmeister_WMDE updated the task description. (Show Details)

Just poked in wikimedia-discovery on IRC.
AFAIK this patch is now ready to be deployed whenever the search platform team is ready.
Once fixed in prod we will schedule the rebuild of released things again (probably for next week)

Restricted Application added a project: User-Addshore. · View Herald TranscriptFri, Nov 22, 12:46 PM

I have deployed Lucas's patch. Kindly test this to confirm everything is fine.

Both vulnerabilities seem to be fixed (T238824 can be tested directly, and I tested this task by running $.i18n.messageStore.messages.en['wdqs-app-resultbrowser-response-summary'] += '<script>alert("hi")</script>' in the browser console), thanks!

In T238822#5688390, @Mathew.onipe wrote:

I have deployed Lucas's patch. Kindly test this to confirm everything is fine.

In T238822#5689217, @Lucas_Werkmeister_WMDE wrote:

Both vulnerabilities seem to be fixed (T238824 can be tested directly, and I tested this task by running $.i18n.messageStore.messages.en['wdqs-app-resultbrowser-response-summary'] += '<script>alert("hi")</script>' in the browser console), thanks!

Great, thanks all. I suppose we'd want to engage in the usual process here (similar to T233213) i.e. 1) make tasks public (not seeing anything to prevent that at first glance) 2) request CVEs. I don't believe there are any additional supported release branches to backport for this?

Addshore added a comment.EditedTue, Nov 26, 9:57 AM

Great, thanks all. I suppose we'd want to engage in the usual process here (similar to T233213) i.e. 1) make tasks public (not seeing anything to prevent that at first glance) 2) request CVEs. I don't believe there are any additional supported release branches to backport for this?

We will do the same process as last time here which involves:

  • Merging some things in gerrit repos
  • Building a new zipped release of the WDQS (with bundleed UI)
  • Triggering new wmde docker image builds
  • Announcing & making the tasks public
  • CVEs, (Still not done for the last ticket yet either, so we can do all 3 at once)

We hope to schedule a time to do all of this sometime in EU daytime this week when @Lea_Lacroix_WMDE is around.

@Lea_Lacroix_WMDE noticed that my fix for T238824 is partially broken:


I’m not sure if we need another security deployment to fix this, though, or if we can push an improved fix to Gerrit (instead of T238824#5681325) and then deploy it normally (once the announcements are over).

Okay, I’ve updated P9720 and uploaded a new deployment patch at /home/lucaswerkmeister-wmde/T238822+T238824-2.patch on deployment.eqiad.wmnet. Note that this is a patch against the upstream deployment repo, so you’d need to first remove the previous patch (without syncing that change) to make it apply, I think.

In Gerrit:

Docker image rebuild https://travis-ci.org/wmde/wikibase-docker/builds/617703004

latest: digest: sha256:6570acb916b429f10ccb3bf3479b66aa6697b3fb3982166a09aba87eeaba7c90
legacy: digest: sha256:4503257bbe1744ce389f07f6dcbaf53db7569cc3e570e30dd5a85c8d0073a39d

Addshore renamed this task from XSS in Wikidata Query Service UI (i18n messages) to XSS in Wikidata Query Service UI (i18n messages) - CVE-2019-19327.Thu, Nov 28, 10:00 AM
Addshore changed the visibility from "Custom Policy" to "Public (No Login Required)".Thu, Nov 28, 10:05 AM
Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptThu, Nov 28, 10:05 AM
Addshore closed this task as Resolved.Thu, Nov 28, 10:05 AM
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.