Page MenuHomePhabricator

Several of the wikieditor-toolbar-* messages are used as raw html
Closed, ResolvedPublic

Description

Not a big deal, but several of the wikieditor-toolbar- (E.g. wikieditor-toolbar-group-format, wikieditor-toolbar-help-heading-description, etc) group of messages are used as raw html. Since this is unnecessary, it would be nice if we could change this to not treat the messages as raw html.

Event Timeline

Bawolff created this task.Jan 9 2017, 6:56 AM
Restricted Application added subscribers: TerraCodes, Aklapper. · View Herald TranscriptJan 9 2017, 6:56 AM
DatGuy triaged this task as Lowest priority.Jan 9 2017, 3:11 PM
DatGuy added a subscriber: DatGuy.

Hey, I'd be interested in working on this since this *should* be easy. Do you mean in the i18n file?

DatGuy added a comment.Jan 9 2017, 4:01 PM
This comment was removed by DatGuy.

Change 331334 had a related patch set uploaded (by DatGuy):
Convert raw html messages to normal messages

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

Change 331334 abandoned by DatGuy:
Convert raw html messages to normal messages

Reason:
After rereviewing this, I believe this is too difficult for me and I do not have the time. Thanks anyways.

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

Change 405729 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/WikiEditor@master] Add support for use of JavaScript Messages API

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

Change 405729 merged by jenkins-bot:
[mediawiki/extensions/WikiEditor@master] Escape messages appropriately and not use raw html

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

D3r1ck01 closed this task as Resolved.
D3r1ck01 claimed this task.
Fomafix reopened this task as Open.Jan 22 2018, 8:30 PM
Fomafix added a subscriber: Fomafix.

Reopened. There are still some system messages that allow HTML and JavaScript injections for users with rights to edit the system messages. For example the message wikieditor-toolbar-help-heading-description.

D3r1ck01 removed D3r1ck01 as the assignee of this task.Jan 25 2018, 12:11 PM
D3r1ck01 added a subscriber: D3r1ck01.

@Fomafix, maybe you could throw more lights? Maybe an example? I can work on this but will need to understand what you mean. Also, if possible, can you update the task description?

Steps to reproduce:

  • Open index.php?title=MediaWiki:Wikieditor-toolbar-help-heading-description&action=edit with an user which has the rights to edit this system message.
  • Insert <i onmouseover="alert('JS injection')">Description</i>.
  • Maybe clear any caches (Shift+Ctrl+R).
  • Hover over the Description in the Help menu.
TheDJ added a subscriber: TheDJ.Jan 25 2018, 9:28 PM

Note that this is likely partially intentional. The dialog config specifically uses the key: "htmlMsg" for these descriptions. That means that likely in some conditions it is expected to actually be HTML. Now getting rid of those expectations might be possible, but I suspect some of it is a bit complicated.

Finding the actual messages (see i18n/en.json) that have this problem should be the first step:
I think that none of the -description ones, actually require HTML (though there are ones with html entities in it that need to be re-encoded/rewritten).

The true purpose is probably the -result messages, which seem to give language specific preview renderings of wikicode syntax. These will need to be separated in the wikieditor logic and be handled separately from the -description and -syntax message. In theory you could replace them with HTML templates and then make those translatable. a bit more work than most cases of these raw html issues.

htmlMsg and textMsg have the same behavior and outputs the raw system message. From security point of view it does not matter if one, some or all messages allow to inject JavaScript code.

@Fomafix, I see that this method defintion wraps the messages in the appropriate message API functions so as not to use raw html messages. Maybe what you are saying is different from this? Or is it related?

autoMsg does not specify if the output contains safe HTML or if the output contains raw text that has to be HTML encoded. It is used for both ways.

Change 458307 had a related patch set uploaded (by Brian Wolff; owner: Jforrester):
[mediawiki/extensions/WikiEditor@master] Fork autoMsg() with escaped autoSafeMsg(), replace where appropriate

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

Change 458307 merged by jenkins-bot:
[mediawiki/extensions/WikiEditor@master] Fork autoMsg() with escaped autoSafeMsg(), replace where appropriate

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

Tgr closed this task as Resolved.Oct 12 2018, 6:31 PM
Tgr assigned this task to Jdforrester-WMF.