Page MenuHomePhabricator

Javascript injection in edit summary on mobile site (CVE-2019-14807)
Closed, ResolvedPublic

Event Timeline

A2093064 created this task.Aug 1 2019, 3:35 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 1 2019, 3:35 AM
MaxSem triaged this task as Unbreak Now! priority.Aug 1 2019, 7:15 AM
MaxSem added projects: MobileFrontend, Vuln-XSS.
MaxSem added a subscriber: MaxSem.

Proposed patch:

This sounds like it needs a CVE?

Looking at this - the XSS only appears to render for me within the first URL. I assume the second URL was provided just to show the diff?

@MaxSem - patch looks sane, going to test it locally now. Trying to think of the best time to security-deploy this. Maybe after the group2 train today but before evening SWAT? Or maybe during the SWAT? Not sure what the protocol is re: security patches getting deployed during SWATs.

I assume the second URL was provided just to show the diff?

Yes.

A2093064 updated the task description. (Show Details)Aug 1 2019, 2:10 PM

Patch reviewed, looks good to me.

Note: I plan to security-deploy @MaxSem's patch from T229541#5383474 this afternoon (Aug 2nd) and provide another update here.

sbassett lowered the priority of this task from Unbreak Now! to High.Aug 2 2019, 9:08 PM
sbassett added a subscriber: Reedy.

Update:

  1. Patch deployed and tested: the above URL no longer renders an XSS for me.
  2. Added this bug to the next security release tracking bug as a sub-task: T225152. I need to double-check w/ @Reedy how we normally do this for deployed extensions. I know that the security release tracking bug is the normal path towards requesting/issuing CVEs, but I know we sometimes just backport as necessary in gerrit for extensions. I'll keep this bug private until I know what to do here.
Reedy added a comment.EditedAug 2 2019, 9:11 PM

We don’t bundle MF so we don’t need to tag it to the next MW release

We can get a CVE for it though if we want

When it’s deployed in production we can just put the patch into master so it will ride the train for new branches. Can even cherry pick to branches and redeploy to make things consistent if you want

@Reedy - ok, thanks. I removed it from T225152. I'll plan to backport in gerrit to master and any necessary MF release branches.

Reedy added a comment.Aug 2 2019, 10:14 PM

Ah yeah, forgot about that. Backporting to supported branches is good :)

Update (I was on vacation earlier this week, just getting back to this):

  1. After chatting w/ @MoritzMuehlenhoff, I've gone ahead and requested a CVE for this vulnerability. I'm not sure we've consistently done this in the past for various MediaWiki extensions (at least for deployed and/or bundled extensions), though I'd like to start doing this on a more consistent basis going forward, perhaps even training other foundation/community folks on the process.
  2. Once I receive confirmation from Mitre of the CVE id, I'll plan to make this task public and backport in gerrit to the 1.31, 1.32 and 1.33 release branches and master, per the current version lifecycle.
  3. I'm not certain any additional communication is warranted here. Posts to phame, wikitech-l, etc. potentially seem to be a little overkill for issues like this and not what's been done in the past. Perhaps the Security-Team should further discuss what might be appropriate messaging for these kinds of vulnerabilities.

Was the patch that we applied directly in production uploaded to gerrit and backported to supported branched already?

@MarcoAurelio - no, was going to start that process now. Wanted to wait until we had a confirmed CVE id.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 9 2019, 6:23 PM
Restricted Application added a subscriber: Cosine02. · View Herald TranscriptAug 9 2019, 6:23 PM

Change 529407 had a related patch set uploaded (by SBassett; owner: MaxSem):
[mediawiki/extensions/MobileFrontend@master] SECURITY: escape edit summaries in feed pages

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

Change 529410 had a related patch set uploaded (by SBassett; owner: MaxSem):
[mediawiki/extensions/MobileFrontend@REL1_33] SECURITY: escape edit summaries in feed pages

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

Change 529411 had a related patch set uploaded (by SBassett; owner: MaxSem):
[mediawiki/extensions/MobileFrontend@REL1_32] SECURITY: escape edit summaries in feed pages

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

Change 529412 had a related patch set uploaded (by SBassett; owner: MaxSem):
[mediawiki/extensions/MobileFrontend@REL1_31] SECURITY: escape edit summaries in feed pages

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

Change 529407 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] SECURITY: escape edit summaries in feed pages

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

Aklapper renamed this task from Javascript injection in edit summary on mobile site to Javascript injection in edit summary on mobile site (CVE-2019-14807).Aug 10 2019, 1:23 PM

Change 529412 merged by SBassett:
[mediawiki/extensions/MobileFrontend@REL1_31] SECURITY: escape edit summaries in feed pages

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

Change 529411 merged by SBassett:
[mediawiki/extensions/MobileFrontend@REL1_32] SECURITY: escape edit summaries in feed pages

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

Change 529410 merged by SBassett:
[mediawiki/extensions/MobileFrontend@REL1_33] SECURITY: escape edit summaries in feed pages

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

sbassett closed this task as Resolved.Aug 14 2019, 3:10 PM
sbassett claimed this task.

I don't believe there's anything else to do here for now.