Merge "Start working on phan-taint-check warnings. Fix minor escaping issues."
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 12 Jul 2018 17:22:14 +0000 (17:22 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 12 Jul 2018 17:22:14 +0000 (17:22 +0000)
13 files changed:
includes/GlobalFunctions.php
includes/Message.php
includes/TemplateParser.php
includes/actions/HistoryAction.php
includes/changes/EnhancedChangesList.php
includes/installer/DatabaseUpdater.php
includes/installer/WebInstaller.php
includes/media/MediaTransformOutput.php
includes/page/ImagePage.php
includes/parser/CoreTagHooks.php
includes/specials/SpecialAutoblockList.php
includes/specials/SpecialMediaStatistics.php
includes/specials/pagers/ImageListPager.php

index 543978b..3ea020f 100644 (file)
@@ -1628,6 +1628,7 @@ function wfClientAcceptsGzip( $force = false ) {
  * As required by the callers, "<nowiki>" is not used.
  *
  * @param string $text Text to be escaped
+ * @param-taint $text escapes_html
  * @return string
  */
 function wfEscapeWikiText( $text ) {
index a499035..5ca0942 100644 (file)
@@ -837,6 +837,7 @@ class Message implements MessageSpecifier, Serializable {
         *   the last time (this is for B/C and should be avoided).
         *
         * @return string HTML
+        * @suppress SecurityCheck-DoubleEscaped phan false positive
         */
        public function toString( $format = null ) {
                if ( $format === null ) {
index 9271e3f..dc7d31e 100644 (file)
@@ -209,8 +209,11 @@ class TemplateParser {
         *     );
         * @endcode
         * @param string $templateName The name of the template
+        * @param-taint $templateName exec_misc
         * @param mixed $args
+        * @param-taint $args none
         * @param array $scopes
+        * @param-taint $scopes none
         * @return string
         */
        public function processTemplate( $templateName, $args, array $scopes = [] ) {
index 8ea50ec..bd64a41 100644 (file)
@@ -836,7 +836,7 @@ class HistoryPager extends ReverseChronologicalPager {
                } else {
                        return MediaWikiServices::getInstance()->getLinkRenderer()->makeKnownLink(
                                $this->getTitle(),
-                               $cur,
+                               new HtmlArmor( $cur ),
                                [],
                                [
                                        'diff' => $this->getWikiPage()->getLatest(),
@@ -868,7 +868,7 @@ class HistoryPager extends ReverseChronologicalPager {
                        # Next row probably exists but is unknown, use an oldid=prev link
                        return $linkRenderer->makeKnownLink(
                                $this->getTitle(),
-                               $last,
+                               new HtmlArmor( $last ),
                                [],
                                [
                                        'diff' => $prevRev->getId(),
@@ -887,7 +887,7 @@ class HistoryPager extends ReverseChronologicalPager {
 
                return $linkRenderer->makeKnownLink(
                        $this->getTitle(),
-                       $last,
+                       new HtmlArmor( $last ),
                        [],
                        [
                                'diff' => $prevRev->getId(),
index a593d27..cdfbf56 100644 (file)
@@ -409,14 +409,14 @@ class EnhancedChangesList extends ChangesList {
 
                # Log timestamp
                if ( $type == RC_LOG ) {
-                       $link = $rcObj->timestamp;
+                       $link = htmlspecialchars( $rcObj->timestamp );
                        # Revision link
                } elseif ( !ChangesList::userCan( $rcObj, Revision::DELETED_TEXT, $this->getUser() ) ) {
-                       $link = '<span class="history-deleted">' . $rcObj->timestamp . '</span> ';
+                       $link = Html::element( 'span', [ 'class' => 'history-deleted' ], $rcObj->timestamp );
                } else {
                        $link = $this->linkRenderer->makeKnownLink(
                                $rcObj->getTitle(),
-                               new HtmlArmor( $rcObj->timestamp ),
+                               $rcObj->timestamp,
                                [],
                                $params
                        );
@@ -642,7 +642,7 @@ class EnhancedChangesList extends ChangesList {
                ];
                // timestamp is not really a link here, but is called timestampLink
                // for consistency with EnhancedChangesListModifyLineData
-               $data['timestampLink'] = $rcObj->timestamp;
+               $data['timestampLink'] = htmlspecialchars( $rcObj->timestamp );
 
                # Article or log link
                if ( $logType ) {
index bf140ea..e49a846 100644 (file)
@@ -208,6 +208,7 @@ abstract class DatabaseUpdater {
         * Output some text. If we're running from web, escape the text first.
         *
         * @param string $str Text to output
+        * @param-taint $str escapes_html
         */
        public function output( $str ) {
                if ( $this->maintenance->isQuiet() ) {
index 2ba221f..739d9f2 100644 (file)
@@ -166,18 +166,7 @@ class WebInstaller extends Installer {
                if ( ( $this->getVar( '_InstallDone' ) || $this->getVar( '_UpgradeDone' ) )
                        && $this->request->getVal( 'localsettings' )
                ) {
-                       $this->request->response()->header( 'Content-type: application/x-httpd-php' );
-                       $this->request->response()->header(
-                               'Content-Disposition: attachment; filename="LocalSettings.php"'
-                       );
-
-                       $ls = InstallerOverrides::getLocalSettingsGenerator( $this );
-                       $rightsProfile = $this->rightsProfiles[$this->getVar( '_RightsProfile' )];
-                       foreach ( $rightsProfile as $group => $rightsArr ) {
-                               $ls->setGroupRights( $group, $rightsArr );
-                       }
-                       echo $ls->getText();
-
+                       $this->outputLS();
                        return $this->session;
                }
 
@@ -1231,6 +1220,25 @@ class WebInstaller extends Installer {
                return WebRequest::detectServer();
        }
 
+       /**
+        * Actually output LocalSettings.php for download
+        *
+        * @suppress SecurityCheck-XSS
+        */
+       private function outputLS() {
+               $this->request->response()->header( 'Content-type: application/x-httpd-php' );
+               $this->request->response()->header(
+                       'Content-Disposition: attachment; filename="LocalSettings.php"'
+               );
+
+               $ls = InstallerOverrides::getLocalSettingsGenerator( $this );
+               $rightsProfile = $this->rightsProfiles[$this->getVar( '_RightsProfile' )];
+               foreach ( $rightsProfile as $group => $rightsArr ) {
+                       $ls->setGroupRights( $group, $rightsArr );
+               }
+               echo $ls->getText();
+       }
+
        /**
         * Output stylesheet for web installer pages
         */
index dba1b60..010cf80 100644 (file)
@@ -507,7 +507,7 @@ class TransformParameterError extends MediaTransformError {
 class TransformTooBigImageAreaError extends MediaTransformError {
        function __construct( $params, $maxImageArea ) {
                $msg = wfMessage( 'thumbnail_toobigimagearea' );
-               $msg->rawParams(
+               $msg->params(
                        $msg->getLanguage()->formatComputingNumbers( $maxImageArea, 1000, "size-$1pixel" )
                );
 
index 81677d4..58f25d4 100644 (file)
@@ -440,7 +440,7 @@ class ImagePage extends Article {
                                                // in the mediawiki.page.image.pagination module
                                                $link = Linker::linkKnown(
                                                        $this->getTitle(),
-                                                       $label,
+                                                       htmlspecialchars( $label ),
                                                        [],
                                                        [ 'page' => $page - 1 ]
                                                );
@@ -460,7 +460,7 @@ class ImagePage extends Article {
                                                $label = $this->getContext()->msg( 'imgmultipagenext' )->text();
                                                $link = Linker::linkKnown(
                                                        $this->getTitle(),
-                                                       $label,
+                                                       htmlspecialchars( $label ),
                                                        [],
                                                        [ 'page' => $page + 1 ]
                                                );
index 438603a..d178600 100644 (file)
@@ -46,6 +46,10 @@ class CoreTagHooks {
         * Text is treated roughly as 'nowiki' wrapped in an HTML 'pre' tag;
         * valid HTML attributes are passed on.
         *
+        * Uses custom html escaping which phan-taint-check won't recognize
+        * hence we suppress the error.
+        * @suppress SecurityCheck-XSS
+        *
         * @param string $text
         * @param array $attribs
         * @param Parser $parser
@@ -75,6 +79,7 @@ class CoreTagHooks {
         *
         * Uses undocumented extended tag hook return values, introduced in r61913.
         *
+        * @suppress SecurityCheck-XSS
         * @param string $content
         * @param array $attributes
         * @param Parser $parser
@@ -110,6 +115,10 @@ class CoreTagHooks {
         *
         * Uses undocumented extended tag hook return values, introduced in r61913.
         *
+        * Uses custom html escaping which phan-taint-check won't recognize
+        * hence we suppress the error.
+        * @suppress SecurityCheck-XSS
+        *
         * @param string $content
         * @param array $attributes
         * @param Parser $parser
index e1909f5..cab5a2e 100644 (file)
@@ -98,7 +98,7 @@ class SpecialAutoblockList extends SpecialPage {
        protected function showTotal( BlockListPager $pager ) {
                $out = $this->getOutput();
                $out->addHTML(
-                       Html::element( 'div', [ 'style' => 'font-weight: bold;' ],
+                       Html::rawElement( 'div', [ 'style' => 'font-weight: bold;' ],
                                $this->msg( 'autoblocklist-total-autoblocks', $pager->getTotalAutoblocks() )->parse() )
                        . "\n"
                );
@@ -119,7 +119,7 @@ class SpecialAutoblockList extends SpecialPage {
                # Not necessary in a standard installation without such extensions enabled
                if ( count( $otherAutoblockLink ) ) {
                        $out->addHTML(
-                               Html::element( 'h2', [], $this->msg( 'autoblocklist-localblocks',
+                               Html::rawElement( 'h2', [], $this->msg( 'autoblocklist-localblocks',
                                        $pager->getNumRows() )->parse() )
                                . "\n"
                        );
index 26c9b06..6dbc09b 100644 (file)
@@ -182,7 +182,7 @@ class MediaStatisticsPage extends QueryPage {
                        [],
                        $linkRenderer->makeLink( $mimeSearch, $mime )
                );
-               $row .= Html::element(
+               $row .= Html::rawElement(
                        'td',
                        [],
                        $this->getExtensionList( $mime )
@@ -245,7 +245,7 @@ class MediaStatisticsPage extends QueryPage {
                $extArray = explode( ' ', $exts );
                $extArray = array_unique( $extArray );
                foreach ( $extArray as &$ext ) {
-                       $ext = '.' . $ext;
+                       $ext = htmlspecialchars( '.' . $ext );
                }
 
                return $this->getLanguage()->commaList( $extArray );
index b2f1487..3d7148d 100644 (file)
@@ -159,6 +159,9 @@ class ImageListPager extends TablePager {
        }
 
        /**
+        * The array keys (but not the array values) are used in sql. Phan
+        * gets confused by this, so mark this method as being ok for sql in general.
+        * @return-taint onlysafefor_sql
         * @return array
         */
        function getFieldNames() {