Page MenuHomePhabricator

Parse warnings shown in plain wikitext with live preview
Closed, ResolvedPublicSecurity

Description

Steps to reproduce:

  • Enable live preview ("Show previews without reloading the page")
  • Open the edit form (e.g. Wikipedia:Sandbox)
  • Pass multiple arguments to the same parameter in a template (e.g. {{tl|1=|1=}}) and hit Show preview

Actual results:

  • The warning box says "Warning: [[:Wikipedia:Sandbox]] is calling [[:Template:Template link]] with more than one value for the "1" parameter. Only the last value provided will be used. ([[Help:Duplicate parameters|Help]])"

Expected results:

  • It says "Warning: Wikipedia:Sandbox is calling Template:Template link with more than one value for the "1" parameter. Only the last value provided will be used. (Help)", as it does with the live preview turned off

I believe line 273 of mediawiki.action.edit.preview.js should be

$previewHeader.find( '.warningbox' ).append( $( '<p>' ).msg( warning ) );

instead of

$previewHeader.find( '.warningbox' ).append( $( '<p>' ).append( warning ) );

though there may be a sleeker solution.

It seems this was introduced in either T190120 or T266311, so pinging their assignees.

Event Timeline

This comment was removed by Nardog.
Nardog renamed this task from Parse warnings shown plain wikitext with live preview to Parse warnings shown in plain wikitext with live preview.Tue, Feb 16, 2:00 PM
matmarex set Security to Software security bug.Tue, Feb 16, 9:28 PM
matmarex added projects: Security, Security-Team.
matmarex changed the visibility from "Public (No Login Required)" to "Custom Policy".
matmarex changed the subtype of this task from "Bug Report" to "Security Issue".

Screenshot:

I think this is a potential XSS issue. The API returns the parse warnings as wikitext, but we treat them as HTML.

This is a simplified version of the API call we make: https://en.wikipedia.org/wiki/Special:ApiSandbox#action=parse&format=json&text=%7B%7Btl%7C1%3D%7C1%3D%7D%7D&prop=text%7Cparsewarnings

I wasn't able to exploit this as a normal user, but I didn't try to test all possible warning messages… I immediately tried this: {{tl|<script>alert()</script>=|<script>alert()</script>=}} and it did not trigger an XSS, because the parameter names go through wfEscapeWikiText(), but it's just a lucky coincidence.

It's definitely exploitable as an interface editor, by editing the relevant localisation messages, e.g. 'duplicate-args-warning' in this case (although this is low severity, we've started treating those as security issues, see T2212).

I have two alternative patches:

Trivial patch to just fix the escaping to resolve the XSS issue (so the warnings are still shown as wikitext, without fixing the original bug):

A bit bigger patch to let the API return the warnings parsed as HTML (this fixes the original bug as well):

The second one should probably be reviewed on Gerrit… I'll submit it there when somebody tells me that's okay.

The bug was introduced in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/597262, which was not in a MediaWiki release yet, so we can probably just deploy the first patch to Wikimedia wikis, and then make this task public again and submit the patches to Gerrit?

The bug was introduced in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/597262, which was not in a MediaWiki release yet, so we can probably just deploy the first patch to Wikimedia wikis, and then make this task public again and submit the patches to Gerrit?

I think that's SOP, yes.

sbassett added a project: SecTeam-Processed.
sbassett added subscribers: Reedy, sbassett.

The bug was introduced in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/597262, which was not in a MediaWiki release yet, so we can probably just deploy the first patch to Wikimedia wikis, and then make this task public again and submit the patches to Gerrit?

As James already noted above, that should be fine. We just had a similar issue where a quasi-security bug (T272770) that hadn't made it into a release was deployed as a security patch and then quickly backported to master. +1 to the append to text patch above; @Reedy or I could likely deploy that today or tomorrow, either before or after the American trains.

sbassett added a parent task: Restricted Task.Mon, Feb 22, 9:07 PM

The first patch (0001-SECURITY-Escape-the-wikitext-of-parse-warning-messag.patch) has been deployed to wmf.31. Logstash seems fine and the issue seems resolved testing the formerly problematic previews on enwiki Wikipedia:Sandbox. I'm going to track this issue on the next security release tracking task, just so we don't forget about it, in case it causes any issues later. @matmarex - it should be fine to push the second patch up to gerrit now, I'll make this task public shortly.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett moved this task from Security Patch To Deploy to Our Part Is Done on the Security-Team board.

Change 666498 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] SECURITY: Escape the wikitext of parse warning messages in live preview

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

Change 666499 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Parse the wikitext of parse warning messages in live preview

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

Change 666498 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: Escape the wikitext of parse warning messages in live preview

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

Change 666499 merged by jenkins-bot:
[mediawiki/core@master] Parse the wikitext of parse warning messages in live preview

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

sbassett assigned this task to matmarex.
sbassett removed a project: Patch-For-Review.