Page MenuHomePhabricator
Create Task
Maniphest T274883

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

Assigned To
matmarex
Authored By
Nardog
Feb 16 2021, 1:56 PM
Tags
Referenced Files
F34108185: 0001-SECURITY-Escape-the-wikitext-of-parse-warning-messag.patch
Feb 16 2021, 9:36 PM
F34108186: 0001-SECURITY-Parse-the-wikitext-of-parse-warning-message.patch
Feb 16 2021, 9:36 PM
F34108178: image.png
Feb 16 2021, 9:28 PM
Subscribers
Aklapper
gerritbot
Jdforrester-WMF
matmarex
Nardog
Reedy
sbassett
TheDJ

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.

Details

SubjectRepoBranchLines +/-
Parse the wikitext of parse warning messages in live previewmediawiki/coremaster+16 -5
SECURITY: Escape the wikitext of parse warning messages in live previewmediawiki/coremaster+1 -1
Customize query in gerrit

Related Objects
Search...

Event Timeline

Nardog created this task.Feb 16 2021, 1:56 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 16 2021, 1:56 PM
Nardog added a comment.Feb 16 2021, 1:59 PM
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.Feb 16 2021, 2:00 PM
matmarex set Security to Software security bug.Feb 16 2021, 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:

image.png (2×3 px, 483 KB)

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).

matmarex added a project: Patch-For-Review.Feb 16 2021, 9:36 PM

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):

0001-SECURITY-Escape-the-wikitext-of-parse-warning-messag.patch1 KBDownload

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

0001-SECURITY-Parse-the-wikitext-of-parse-warning-message.patch2 KBDownload

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

matmarex added a comment.Feb 16 2021, 9:40 PM

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?

Jdforrester-WMF subscribed.Feb 17 2021, 12:55 AM
In T274883#6835461, @matmarex wrote:

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 moved this task from Incoming to Security Patch To Deploy on the Security-Team board.Feb 17 2021, 3:50 PM
sbassett added a project: SecTeam-Processed.
sbassett added subscribers: Reedy, sbassett.
In T274883#6835461, @matmarex wrote:

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: T270459: Tracking bug for MediaWiki 1.31.13/1.35.2.Feb 22 2021, 9:07 PM
sbassett triaged this task as Low priority.Feb 22 2021, 9:10 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 mentioned this in T270459: Tracking bug for MediaWiki 1.31.13/1.35.2.Feb 22 2021, 9:11 PM
sbassett removed a project: Patch-For-Review.
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.
gerritbot added a comment.Feb 23 2021, 11:43 PM

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

gerritbot added a project: Patch-For-Review.Feb 23 2021, 11:43 PM

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

gerritbot added a comment.Feb 24 2021, 12:10 AM

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.33; 2021-03-02).Feb 24 2021, 1:00 AM
gerritbot added a comment.Feb 26 2021, 12:18 AM

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 closed this task as Resolved.Feb 26 2021, 8:22 PM
sbassett assigned this task to matmarex.
sbassett removed a project: Patch-For-Review.
Reedy added a subscriber: gerritbot.Apr 8 2021, 7:11 PM
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