Wikimedia

Fix the dangerous API in WikiImporter::notice echoing of unescaped values

The way notices work in the WikiImporter class of MediaWiki core is sketchy. It's 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 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 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.

Task tags

  • security
  • php
  • inheritance

Students who completed this task

Yifei He

Task type

  • code Code
close

2017