Change Details

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.

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

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