Page MenuHomePhabricator

Add phan-taint-check-plugin to Flow extension
Closed, ResolvedPublic

Description

Would be nice to add phan-taint-check-plugin to Flow extensions

<?xml version="1.0" encoding="ISO-8859-15"?>
<checkstyle version="6.5">
  <file name="./Hooks.php">
    <error line="1371" severity="warning" message="Calling method \MediaWiki\Linker\LinkRenderer::makeLink() in \FlowHooks::onWatchlistEditorBuildRemoveLine that outputs using tainted argument $content. (Caused by: Builtin-\MediaWiki\Linker\LinkRenderer::makeLink)" source="SecurityCheck-DoubleEscaped"/>
  </file>
  <file name="./includes/Dump/Exporter.php">
    <error line="430" severity="warning" message="Calling method \Xml::element() in \Flow\Dump\Exporter::formatRevision that outputs using tainted argument $[arg #3]. (Caused by: Builtin-\Xml::element)" source="SecurityCheck-DoubleEscaped"/>
    <error line="430" severity="warning" message="Calling method \Xml::element() in \Flow\Dump\Exporter::formatRevision that outputs using tainted argument $attribs. (Caused by: Builtin-\Xml::element)" source="SecurityCheck-DoubleEscaped"/>
  </file>
  <file name="./includes/Formatter/CategoryViewerFormatter.php">
    <error line="28" severity="warning" message="Calling method \MediaWiki\Linker\LinkRenderer::makeLink() in \Flow\Formatter\CategoryViewerFormatter::format that outputs using tainted argument $[arg #2]. (Caused by: Builtin-\MediaWiki\Linker\LinkRenderer::makeLink)" source="SecurityCheck-DoubleEscaped"/>
  </file>
  <file name="./includes/Log/ActionFormatter.php">
    <error line="126" severity="warning" message="Calling method \Flow\Log\ActionFormatter::getPerformerElement in \Flow\Log\ActionFormatter::getActionMessage that is always unsafe " source="SecurityCheck-DoubleEscaped"/>
    <error line="128" severity="warning" message="Calling method \Flow\Log\ActionFormatter::getPerformerElement in \Flow\Log\ActionFormatter::getActionMessage that is always unsafe " source="SecurityCheck-DoubleEscaped"/>
    <error line="163" severity="warning" message="Calling method \LogFormatter::getActionText in \Flow\Log\ActionFormatter::getActionText that is always unsafe  (Caused by: ../../includes/logging/LogFormatter.php +443)" source="SecurityCheck-DoubleEscaped"/>
  </file>
  <file name="./includes/SpamFilter/SpamBlacklist.php">
    <error line="55" severity="warning" message="Calling method \Parser::parse() in \Flow\SpamFilter\SpamBlacklist::getLinks that outputs using tainted argument $[arg #1]. (Caused by: Builtin-\Parser::parse)" source="SecurityCheck-DoubleEscaped"/>
  </file>
  <file name="./includes/TemplateHelper.php">
    <error line="407" severity="warning" message="Calling method \htmlspecialchars() in \Flow\TemplateHelper::historyTimestamp that outputs using tainted argument $formattedTime." source="SecurityCheck-DoubleEscaped"/>
  </file>
  <file name="./maintenance/FlowFixLog.php">
    <error line="102" severity="warning" message="Calling method \FlowFixLog::error() in \LogRowUpdateGenerator::update that outputs using tainted argument $[arg #1]. (Caused by: ./maintenance/FlowFixLog.php +80)" source="SecurityCheck-XSS"/>
    <error line="117" severity="warning" message="Calling method \FlowFixLog::error() in \LogRowUpdateGenerator::update that outputs using tainted argument $[arg #1]. (Caused by: ./maintenance/FlowFixLog.php +80)" source="SecurityCheck-XSS"/>
    <error line="126" severity="warning" message="Calling method \FlowFixLog::error() in \LogRowUpdateGenerator::update that outputs using tainted argument $[arg #1]. (Caused by: ./maintenance/FlowFixLog.php +80)" source="SecurityCheck-XSS"/>
  </file>
  <file name="./maintenance/repair_missing_revision_content.php">
    <error line="178" severity="warning" message="Echoing expression that was not html escaped (Caused by: ./scripts/generatecss.php +25; ./scripts/generatecss.php +6; ./maintenance/repair_missing_revision_content.php +177)" source="SecurityCheck-XSS"/>
    <error line="187" severity="warning" message="Echoing expression that was not html escaped (Caused by: ./scripts/generatecss.php +25; ./scripts/generatecss.php +6; ./maintenance/repair_missing_revision_content.php +177; ./maintenance/repair_missing_revision_content.php +186; ./maintenance/repair_missing_revision_content.php +185)" source="SecurityCheck-XSS"/>
    <error line="238" severity="warning" message="Echoing expression that was not html escaped (Caused by: ./maintenance/repair_missing_revision_content.php +159; ./scripts/generatecss.php +25; ./scripts/generatecss.php +6; ./maintenance/repair_missing_revision_content.php +208; ./maintenance/repair_missing_revision_content.php +231; ./maintenance/repair_missing_revision_content.php +230)" source="SecurityCheck-XSS"/>
  </file>
  <file name="./scripts/generatecss.php">
    <error line="26" severity="warning" message="Echoing expression that was not html escaped (Caused by: ./scripts/generatecss.php +25; ./scripts/generatecss.php +6)" source="SecurityCheck-XSS"/>
  </file>
</checkstyle>

\Flow\Model\AbstractRevision::getContent is similar to T183174, because it has a parameter so it can return html or wikitext, which makes it hard to detect and cause some failures here

ActionFormatter.php looks like the problem in T201565

TemplateHelper.php looks like var reuse

\FlowFixLog::error refers to Maintenance::error, which should be safe

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 460466 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/extensions/Flow@master] SECURITY: Make generatecss.php command line only & make phan-taint-check pass

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

Change 460466 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] SECURITY: Make generatecss.php command line only & make phan-taint-check pass

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

Legoktm assigned this task to Bawolff.
sbassett triaged this task as Medium priority.Oct 15 2019, 7:33 PM