I'm on mobile and am unable to confirm this issue. Could someone else take a look? Otherwise, I'll be back in front of my laptop shortly and will investigate then. Also, apologies for the odd formatting of the description.

looking. note I don't think Parsoid is exposed directly to the internet in WMF production, only via RB?

Looks like this code may be implicated as I don't see entity encoding happening here:

https://github.com/wikimedia/parsoid/blob/06bf9cea8b80598ecdb7fe708f222820946ce70f/lib/api/routes.js#L40

In T149504#2754642, @Krenair wrote:

looking. note I don't think Parsoid is exposed directly to the internet in WMF production, only via RB?

I'm guessing the reporter found this as part of a pen test, either internal to their org or for a customer.

The version of Parsoid running on deployment-parsoid09.deployment-prep.eqiad.wmflabs does not appear to be vulnerable to this, it returns Cannot GET /%3Cimg%20src=x%20onerror%22javascript:alert('XSS')%22%3E

In T149504#2754646, @Krenair wrote:

The version of Parsoid running on deployment-parsoid09.deployment-prep.eqiad.wmflabs does not appear to be vulnerable to this, it returns Cannot GET /%3Cimg%20src=x%20onerror%22javascript:alert('XSS')%22%3E

I believe that the reproduction URL in the text of the original report is incorrect, but the screenshot has the correct repro URL. See the screenshot included in the original e-mail to security@ if you have access to that. Also, can you attach that image to this bug description? Thanks! (I'm still on mobile.)

Adding original bug reporter as a subscriber to this issue.

In T149504#2754648, @dpatrick wrote:

See the screenshot included in the original e-mail to security@ if you have access to that. Also, can you attach that image to this bug description? Thanks!

I don't. I'm not sure why I don't, given that I am in the Security group in Phabricator.

In T149504#2754651, @Krenair wrote:

In T149504#2754648, @dpatrick wrote:

See the screenshot included in the original e-mail to security@ if you have access to that. Also, can you attach that image to this bug description? Thanks!

I don't. I'm not sure why I don't, given that I am in the Security group in Phabricator.

@dpatrick do you want ops to change that and add @Krenair? Any other changes to that mail alias ? Maybe really quick separate ticket? it's in the private repo who is getting mail to that alias, but theoretically it could also be moved to public repo or google group

There is a quick simple fix for this which addresses errors from all api endpoints (other endpoints are vulnerable to this as well).

diff --git a/lib/api/routes.js b/lib/api/routes.js
index 9a56651..7b4b0ec 100644
--- a/lib/api/routes.js
+++ b/lib/api/routes.js
@@ -14,6 +14,7 @@ var MWParserEnv = require('../config/MWParserEnvironment.js').MWParserEnvironmen
 var Promise = require('../utils/promise.js');
 var LogData = require('../logger/LogData.js').LogData;
 var ApiRequest = require('../mw/ApiRequest.js');
+var Util = require('../utils/Util.js').Util;
 
 var TemplateRequest = ApiRequest.TemplateRequest;
 
@@ -31,7 +32,7 @@ module.exports = function(parsoidConfig, processLogger) {
                err.httpStatus = httpStatus || 404;
                err.suppressLoggingStack = true;
                processLogger.log('fatal/request', err);
-               apiUtils.sendResponse(res, {}, text, err.httpStatus);
+               apiUtils.sendResponse(res, { log: function() {} }, Util.entityEncodeAll(text), err.httpStatus);
        };

But what is the protocol for these fixes? Patch these immediately privately? Fix them by pushing them to gerrit and go through usual review and then deploy?

The severity is a bit lower for 2 reasons:
(a) Parsoid is not exposed publicly and is proxied by RESTBase.
(b) Parsoid has no access to user credentials and is a stateless service (except when VE is requesting an edit on a private wiki, but since VE is doing these requests, this exploit is not exposed, and since Parsoid is stateless, the cookie doesn't stick around in the server).

So, my question is if this can wait till Monday when Arlo and I can review code to see if this leaks anywhere else and fix everything in one shot?

Original comment was removed accidentally.

Hi everybody,

Sorry for the vague details on my original mail, I'll try to expand a little bit. This issue is triggered when calling the Round-trip pages test feature (haven't tested other features) with the XSS payload as prefix.

As @Krenair said, the Parsoid web service for MediaWiki is not accessible from the internet, although that may be the case in internal environments where private wikis are using the service.

I'm also uploading the original screenshot I've attached in the report.

It is close to midnight, and I am heading to bed now, but will check messages tomorrow morning. But, the above diff will fix this and similar XSS vulnerabilities in other endpoints.

Unless there is a recommendation to deploy this tomorrow, we'll patch the cluster on Monday. At that time, we will also release a new deb package. I'll greatly appreciate any guidance with process and protocol for doing this properly.

In T149504#2754833, @ssastry wrote:

Unless there is a recommendation to deploy this tomorrow, we'll patch the cluster on Monday. At that time, we will also release a new deb package. I'll greatly appreciate any guidance with process and protocol for doing this properly.

It's fine to wait until Monday to investigate more thoroughly, then fix this internally and release updated packages. Since this is not bundled with core MediaWiki, you can bundle the fix and announce it's availability through whichever channels you use for normal releases of Parsoid (e.g. wikitech-l).

Also, if there are any versions of Parsoid that are still supported besides trunk, you'll need to backport fixes and create releases for those versions as well.

I can give you more details about this process on Monday morning PDT if you need.

(b) Parsoid has no access to user credentials and is a stateless service

Yeah, but we're also running it like this,
https://parsoid-tests.wikimedia.org/parsoid/
which seems pretty bad.

https://parsoid-tests.wikimedia.org/parsoid/

I stopped the service on ruthenium. But that doesn't help other people running it like that.

I don't think anything trusts the whole of *.wikimedia.org, that contains third party hosted services like the blog

@dpatrick, can you also tell me what the protocol is for security fixes for the cluster? Patch them privately and then upload gerrit patches? Or fix them by pushing them to gerrit and go through usual review and then deploy? I presume this is dependent on the severity of the discovered exploit .. but would be helpful to know.

Patch them, put patch here for review, when approved deploy privately in production, only upload to gerrit as part of the public security release

Got it. Will deploy patch in T149504#2754820 at 1pm PT.

@ssastry

1. Develop a fix for the issue, then attach your patch (as a .patch file that cleanly applies to master) to a comment on this ticket. I'll review and test the patch to confirm that the problem is fully addressed.
2. Deploy the patch to the cluster.
3. Prepare an announcement to the community describing the issue and the appropriate fix.
4. Send the announcement and concurrently upload and merge the patch through Gerrit.

@ssastry, see Gergõ's recent messages, dated Oct. 27th, to mediawiki-announce for an idea of how to format your announcement. You can send it using whichever lists you announce normal releases of Parsoid.

In T149504#2757148, @dpatrick wrote:

@ssastry

1. Develop a fix for the issue, then attach your patch (as a .patch file that cleanly applies to master) to a comment on this ticket. I'll review and test the patch to confirm that the problem is fully addressed.

See

Added @GWicke and @mobrovac as an FYI just in case something like this is exploitable via RESTBase or RESTBase has a similar exploit.

RESTBase formats all its errors as https://tools.ietf.org/html/draft-nottingham-http-problem-07 (JSON), so should not be vulnerable.

In T149504#2757180, @ssastry wrote:

In T149504#2757148, @dpatrick wrote:

@ssastry

1. Develop a fix for the issue, then attach your patch (as a .patch file that cleanly applies to master) to a comment on this ticket. I'll review and test the patch to confirm that the problem is fully addressed.

See

t149504.patch857 BDownload

. Please apply this to 63f1e1512785d31a15f97210fce14b715dfd1a95 which is the current deployed version on the cluster. The patch also applies to master, but in production, this patch will be applied to 63f1e1512 instead.

@Arlolra is reviewing this patch and the exploit independently and looking for any other code paths that expose this.

Here is the proposed language for the security announcement:

A security bug in Parsoid has been fixed [1].

* Users could send invalid prefixes, formats, or domains and run
  javascript code on the error page that Parsoid displayed.

* This fix has been applied to the Wikimedia cluster and also merged
  into Parsoid master. We are going to release a 0.5.2 deb version 
  later today with this patch applied.

* Parsoid is a stateless service and doesn't retain any state between 
  requests. In private wikis, VisualEditor can be configured to
  forward the user cookie to Parsoid to pass along to the MediaWiki API 
  to parse a page, but this exploit is not exposed through VE.

* In addition, Parsoid doesn't receive any user credentials in public wikis.
  However, if a wiki's Parsoid service is publicly accessible on the internet
  *and* is accessible through the wiki's domain, then, this exploit can be used
  to leak user cookies for that wiki. For all wikis that use Parsoid 
  in this fashion, we recommend they patch their Parsoid installation 
  immediately.

In T149504#2757517, @ssastry wrote:

Here is the proposed language for the security announcement:

• This fix has been applied to the Wikimedia cluster and also merged into Parsoid master. We are going to release a 0.5.2 deb version later today with this patch applied.

Can you time transmission of your announcement to be after the deb is available so that users can immediately install the deb?

In T149504#2757522, @dpatrick wrote:

In T149504#2757517, @ssastry wrote:

Here is the proposed language for the security announcement:

• This fix has been applied to the Wikimedia cluster and also merged into Parsoid master. We are going to release a 0.5.2 deb version later today with this patch applied.

Can you time transmission of your announcement to be after the deb is available so that users can immediately install the deb?

Will do.

In T149504#2755476, @Arlolra wrote:

https://parsoid-tests.wikimedia.org/parsoid/

I stopped the service on ruthenium. But that doesn't help other people running it like that.

@Arlolra Thanks for disabling that. That would have allowed attackers access to the non-HttpOnly "GeoIP" cookie scoped to .wikimedia.org.

I think the above provided patch is incomplete. It misses some error responses going through errBack, which, incidentally, should be the majority of them.

Here's slightly more thorough approach,

Sorry, that patch need rebasing on 63f1e151. One sec.

Orthogonally, we shouldn't even be exposing these development routes in production, https://gerrit.wikimedia.org/r/#/c/318972

This one should apply cleanly,

In T149504#2757806, @Arlolra wrote:

This one should apply cleanly,

0001-T149504.patch3 KBDownload

Okay, LGTM. This patch applies even closer to the output (sendResponse) than my patch and should also protect all future API endpoints and changes. I ran parser tests, roundtrip tests, and verified that the exploits are no longer there.

In T149504#2757806, @Arlolra wrote:

This one should apply cleanly,

0001-T149504.patch3 KBDownload

This needs a rebase on top of 61517a2f61a1ac4ec9691a877ab347aa65e62c16.

In T149504#2757959, @dpatrick wrote:

In T149504#2757806, @Arlolra wrote:

This one should apply cleanly,

0001-T149504.patch3 KBDownload

This needs a rebase on top of 61517a2f61a1ac4ec9691a877ab347aa65e62c16.

But, we aren't deploying on top of that. We are patching code that is in production which is 63f1e1512.

This needs a rebase on top of 61517a2f61a1ac4ec9691a877ab347aa65e62c16.

Here you go

The previous one applies to the currently deployed commit,
https://www.mediawiki.org/wiki/Parsoid/Deployments#Monday.2C_October_24.2C_2016_around_1:15_PT:_Y.C2.A0Deployed_63f1e151

@ssastry, in the announcement you might want to note that the WMF infrastructure does not directly expose Parsoid, and was not affected.

In T149504#2757990, @Arlolra wrote:

This needs a rebase on top of 61517a2f61a1ac4ec9691a877ab347aa65e62c16.

Here you go

0001-T149504-Fix-reflected-XSS.patch3 KBDownload

. This applied to HEAD of master.

The previous one applies to the currently deployed commit,
https://www.mediawiki.org/wiki/Parsoid/Deployments#Monday.2C_October_24.2C_2016_around_1:15_PT:_Y.C2.A0Deployed_63f1e151

Thanks. One other small change. In the body of your commit message, please add "Bug: T149504" on a line by itself, and prefix the subject line of the commit with "SECURITY: ".

Aside from those changes. The patch looks good and appropriately mitigates the problem, based on testing. Please go ahead and deploy internally and release to the community when you are ready.

In T149504#2758000, @GWicke wrote:

@ssastry, in the announcement you might want to note that the WMF infrastructure does not directly expose Parsoid, and was not affected.

Mostly true, but see T149504#2755475 and followup comments.

In T149504#2758036, @ssastry wrote:

In T149504#2758000, @GWicke wrote:

@ssastry, in the announcement you might want to note that the WMF infrastructure does not directly expose Parsoid, and was not affected.

Mostly true, but see T149504#2755475 and followup comments.

As a user of WMF infrastructure, I care about whether this exploit was usable to steal my sessions. As *.wikimedia.org is not trusted for session data, I think the answer to this question is "no", and it would probably be helpful to some users to mention this.

In T149504#2755475, @Arlolra wrote:

(b) Parsoid has no access to user credentials and is a stateless service

Yeah, but we're also running it like this,
https://parsoid-tests.wikimedia.org/parsoid/
which seems pretty bad.

Maybe a good reason to see if we can assign a non-wmf domain name for our testing services.

In T149504#2758065, @GWicke wrote:

In T149504#2758036, @ssastry wrote:

In T149504#2758000, @GWicke wrote:

@ssastry, in the announcement you might want to note that the WMF infrastructure does not directly expose Parsoid, and was not affected.

Mostly true, but see T149504#2755475 and followup comments.

As a user of WMF infrastructure, I care about whether this exploit was usable to steal my sessions. As *.wikimedia.org is not trusted for session data, I think the answer to this question is "no", and it would probably be helpful to some users to mention this.

Agreed. I can add comments about not compromising user sessions.

Separately, should testreduce even running on a production server/host? I doubt it has ever been audited.

/cc @Dzahn

Ok, time for a lunch break. Arlo and I will co-ordinate on rest of the steps and update the todo list here as we get through it.

In T149504#2758124, @Arlolra wrote:

Separately, should testreduce even running on a production server/host? I doubt it has ever been audited.

/cc @Dzahn

I don't know much about that, but it seems like yes, it should run in production from what i can see on https://rt.wikimedia.org/Ticket/Display.html?id=6980 (this ticket has not been imported to Phabricator as it's a procurement ticket). It was argued and approved there that it should run in prod, not in labs. cc: @faidon

In T149504#2757517, @ssastry wrote:

• This fix has been applied to the Wikimedia cluster and also merged into Parsoid master. We are going to release a 0.5.2 deb version later today with this patch applied.

This should say 0.5.3 right?

In T149504#2758551, @Legoktm wrote:

In T149504#2757517, @ssastry wrote:

• This fix has been applied to the Wikimedia cluster and also merged into Parsoid master. We are going to release a 0.5.2 deb version later today with this patch applied.

This should say 0.5.3 right?

Yes.

In T149504#2758538, @Dzahn wrote:

In T149504#2758124, @Arlolra wrote:

Separately, should testreduce even running on a production server/host? I doubt it has ever been audited.

/cc @Dzahn

I don't know much about that, but it seems like yes, it should run in production from what i can see on https://rt.wikimedia.org/Ticket/Display.html?id=6980 (this ticket has not been imported to Phabricator as it's a procurement ticket). It was argued and approved there that it should run in prod, not in labs. cc: @faidon

As of December, I had wanted it non-labs hardware because we are running lots of tests and we want tests to finish in a reasonable amount of time + I wanted to use perf numbers to detect performance regressions before deploys (which I haven't made use of as much as I wanted to).

But, would it be possible to assign ruthenium a non-wikimedia.org hostname?

Just an update for those following along. @Dzahn is helping with debian package upload and troubleshooting since the package isn't getting uploaded properly. Once that is done, we'll proceed with next steps.

Update: After more than a couple of hours of wrangling with reprepro, etc. @Dzahn and i decided to pick this up tomorrow with Moritz. Once the debian upload succeeds tomorrow, we'll proceed with the other steps and I'll send out an announcement after they are all done.

https://lists.wikimedia.org/pipermail/wikitech-l/2016-November/086904.html for the record. :)

Can we continue the discussion of the remaining unresolved piece on this ticket: is there a way to use a non-wm.o domain name for ruthenium? Or, should I open a new security ticket for it?

Maybe it could go under another domain like wmfusercontent.org?

Hi everybody,

As patch has been deployed and announced I would like to request a CVE id with the issue. Is this okay or would you like for me to hold off the request for more time?

Thanks in advance,

This appears to have been fixed, and should probably be marked as resolved and made public?

In T149504#2761110, @nmiguez wrote:

Hi everybody,

As patch has been deployed and announced I would like to request a CVE id with the issue. Is this okay or would you like for me to hold off the request for more time?

Thanks in advance,

I am fine with it. I'll let @dpatrick respond as well.

In T149504#2760949, @Krenair wrote:

Maybe it could go under another domain like wmfusercontent.org?

Yup, that is one possibility. Anyway, looks like this can be a different ticket at this point. Separately, @Legoktm was asking why the geoip cookie is being shared with *.wikimedia.org in the first place. If that weren't the case, there is no harm using parsoid-tests.wikimedia.org domain.

In T149504#2762762, @matmarex wrote:

This appears to have been fixed, and should probably be marked as resolved and made public?

I'll let @dpatrick make a call on making it public by looking at the discussion on this ticket.

In T149504#2761110, @nmiguez wrote:

Hi everybody,

As patch has been deployed and announced I would like to request a CVE id with the issue. Is this okay or would you like for me to hold off the request for more time?

Thanks in advance,

Requesting a CVE is fine. Please CC me (dpatrick@wikimedia.org) on your request. Alternatively, I can make the request and CC you, using verbiage that you are amenable to. Either is fine with me.

In T149504#2766951, @dpatrick wrote:

In T149504#2761110, @nmiguez wrote:

Hi everybody,

As patch has been deployed and announced I would like to request a CVE id with the issue. Is this okay or would you like for me to hold off the request for more time?

Thanks in advance,

Requesting a CVE is fine. Please CC me (dpatrick@wikimedia.org) on your request. Alternatively, I can make the request and CC you, using verbiage that you are amenable to. Either is fine with me.

@dpatrick CVE Request are no longer done by email, but rather using a web form (https://cveform.mitre.org/). If you want to request it, that would be best for me since you are more familiarized with the software versions. Please set the credits for Nicolás Miguez @ Deloitte.

Thanks in advance,