Page MenuHomePhabricator

WikiImporter::notice echoing of unescaped values is a dangerous api
Closed, ResolvedPublic


The way notices work in the WikiImporter class is really sketchy. Its safe in MediaWiki, but it seems risky that someone may use it in an unsafe manner, and thus should be fixed.

WikiImporter::notice is used to report notices back to the user. It can be overriden via setNoticeCallback() in order to have custom reporting. However it has a default implementation of just echoing out the notice.

This is very risky, since the notice contains user controlled variables, and no escaping is applied. The rationale is that its meant for the command line where this makes sense. However if it was triggered in the web interface (and no output buffer clearing took place) then this would be an XSS.

In MediaWiki, Special:Import overrides the notice callback, so its safe. The API does not, however the api clears the output buffer so it is safe.

Nonetheless this is really sketchy, and should be changed so that nobody in the future accidentally causes a security issue. The two possibilities:

  • Either make the default notice implementation just send the notice to wfDebug(), and require that the command line importers call setNoticeCallback() for their own custom callback to echo the notice. [I think this is the better option]
  • Or make the default implementation check $wgCommandLineMode, and output the message as ->escaped() if not in command line mode.

As an aside, this was found using an experimental custom phan plugin I'm working on

Event Timeline

Bawolff created this task.Oct 11 2017, 8:18 PM
Restricted Application added subscribers: TerraCodes, Aklapper. · View Herald TranscriptOct 11 2017, 8:18 PM
dpatrick triaged this task as Medium priority.Oct 12 2017, 4:20 PM
Bawolff updated the task description. (Show Details)Oct 12 2017, 4:21 PM

Change 399770 had a related patch set uploaded (by Eflyjason; owner: Eflyjason):
[mediawiki/core@master] [WikiImporter::notice] use wfDebug instead of echo in notice()

Florian closed this task as Resolved.Dec 22 2017, 11:28 AM
Florian assigned this task to eflyjason.

Change 399770 merged by jenkins-bot:
[mediawiki/core@master] [WikiImporter::notice] use wfDebug instead of echo in notice()