SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
authorBrian Wolff <bawolff+wn@gmail.com>
Mon, 6 Feb 2017 03:00:39 +0000 (03:00 +0000)
committerBrian Wolff <bawolff+wn@gmail.com>
Tue, 28 Mar 2017 21:51:44 +0000 (21:51 +0000)
System messages may take parameters from untrusted sources. This
may include taking parameters from urls given by unauthenticated
users even if the wiki is a read-only wiki. Allowing <html> tags
in such a context seems like an accident waiting to happen.

Bug: T156184
Change-Id: I661f482986d319cf41da1d3e7b20a0f028a42e90

includes/OutputPage.php
includes/cache/MessageCache.php
includes/parser/CoreTagHooks.php
includes/parser/ParserOptions.php
languages/i18n/en.json
languages/i18n/qqq.json
tests/phpunit/includes/MessageTest.php

index 9ecfa23..d3e1373 100644 (file)
@@ -1568,6 +1568,7 @@ class OutputPage extends ContextSource {
                        // been changed somehow, and keep it if so.
                        $anonPO = ParserOptions::newFromAnon();
                        $anonPO->setEditSection( false );
+                       $anonPO->setAllowUnsafeRawHtml( false );
                        if ( !$options->matches( $anonPO ) ) {
                                wfLogWarning( __METHOD__ . ': Setting a changed bogus ParserOptions: ' . wfGetAllCallers( 5 ) );
                                $options->isBogus = false;
@@ -1581,6 +1582,7 @@ class OutputPage extends ContextSource {
                                // either.
                                $po = ParserOptions::newFromAnon();
                                $po->setEditSection( false );
+                               $po->setAllowUnsafeRawHtml( false );
                                $po->isBogus = true;
                                if ( $options !== null ) {
                                        $this->mParserOptions = empty( $options->isBogus ) ? $options : null;
@@ -1590,6 +1592,7 @@ class OutputPage extends ContextSource {
 
                        $this->mParserOptions = ParserOptions::newFromContext( $this->getContext() );
                        $this->mParserOptions->setEditSection( false );
+                       $this->mParserOptions->setAllowUnsafeRawHtml( false );
                }
 
                if ( $options !== null && !empty( $options->isBogus ) ) {
index 7cd489a..70e1d9a 100644 (file)
@@ -191,11 +191,16 @@ class MessageCache {
                                // either.
                                $po = ParserOptions::newFromAnon();
                                $po->setEditSection( false );
+                               $po->setAllowUnsafeRawHtml( false );
                                return $po;
                        }
 
                        $this->mParserOptions = new ParserOptions;
                        $this->mParserOptions->setEditSection( false );
+                       // Messages may take parameters that could come
+                       // from malicious sources. As a precaution, disable
+                       // the <html> parser tag when parsing messages.
+                       $this->mParserOptions->setAllowUnsafeRawHtml( false );
                }
 
                return $this->mParserOptions;
index c943b7c..438603a 100644 (file)
@@ -79,12 +79,25 @@ class CoreTagHooks {
         * @param array $attributes
         * @param Parser $parser
         * @throws MWException
-        * @return array
+        * @return array|string Output of tag hook
         */
        public static function html( $content, $attributes, $parser ) {
                global $wgRawHtml;
                if ( $wgRawHtml ) {
-                       return [ $content, 'markerType' => 'nowiki' ];
+                       if ( $parser->getOptions()->getAllowUnsafeRawHtml() ) {
+                               return [ $content, 'markerType' => 'nowiki' ];
+                       } else {
+                               // In a system message where raw html is
+                               // not allowed (but it is allowed in other
+                               // contexts).
+                               return Html::rawElement(
+                                       'span',
+                                       [ 'class' => 'error' ],
+                                       // Using ->text() not ->parse() as
+                                       // a paranoia measure against a loop.
+                                       wfMessage( 'rawhtml-notallowed' )->escaped()
+                               );
+                       }
                } else {
                        throw new MWException( '<html> extension tag encountered unexpectedly' );
                }
index 7be8281..2cdd8c7 100644 (file)
@@ -243,6 +243,21 @@ class ParserOptions {
         */
        private $redirectTarget = null;
 
+       /**
+        * If the wiki is configured to allow raw html ($wgRawHtml = true)
+        * is it allowed in the specific case of parsing this page.
+        *
+        * This is meant to disable unsafe parser tags in cases where
+        * a malicious user may control the input to the parser.
+        *
+        * @note This is expected to be true for normal pages even if the
+        *  wiki has $wgRawHtml disabled in general. The setting only
+        *  signifies that raw html would be unsafe in the current context
+        *  provided that raw html is allowed at all.
+        * @var boolean
+        */
+       private $allowUnsafeRawHtml = true;
+
        public function getInterwikiMagic() {
                return $this->mInterwikiMagic;
        }
@@ -457,6 +472,15 @@ class ParserOptions {
        public function getMagicRFCLinks() {
                return $this->mMagicRFCLinks;
        }
+
+       /**
+        * @since 1.29
+        * @return bool
+        */
+       public function getAllowUnsafeRawHtml() {
+               return $this->allowUnsafeRawHtml;
+       }
+
        public function setInterwikiMagic( $x ) {
                return wfSetVar( $this->mInterwikiMagic, $x );
        }
@@ -596,6 +620,15 @@ class ParserOptions {
                return wfSetVar( $this->mIsPrintable, $x );
        }
 
+       /**
+        * @param bool|null Value to set or null to get current value
+        * @return bool Current value for allowUnsafeRawHtml
+        * @since 1.29
+        */
+       public function setAllowUnsafeRawHtml( $x ) {
+               return wfSetVar( $this->allowUnsafeRawHtml, $x );
+       }
+
        /**
         * Set the redirect target.
         *
index 0c8bf21..f9fbfd0 100644 (file)
        "restrictionsfield-label": "Allowed IP ranges:",
        "restrictionsfield-help": "One IP address or CIDR range per line. To enable everything, use:<pre>0.0.0.0/0\n::/0</pre>",
        "revid": "revision $1",
-       "pageid": "page ID $1"
+       "pageid": "page ID $1",
+       "rawhtml-notallowed": "&lt;html&gt; tags cannot be used outside of normal pages."
 }
index 2951a4c..75be7aa 100644 (file)
        "restrictionsfield-label": "Field label shown for restriction fields (e.g. on Special:BotPassword).",
        "restrictionsfield-help": "Placeholder text displayed in restriction fields (e.g. on Special:BotPassword).",
        "revid": "Used to format a revision ID number in text. Parameters:\n* $1 - Revision ID number.\n{{Identical|Revision}}",
-       "pageid": "Used to format a page ID number in text. Parameters:\n* $1 - Page ID number."
+       "pageid": "Used to format a page ID number in text. Parameters:\n* $1 - Page ID number.",
+       "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an &lt;html&gt; tag in a system message or somewhere other than a normal page."
 }
index 424218e..58087c1 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+use MediaWiki\MediaWikiServices;
 
 class MessageTest extends MediaWikiLangTestCase {
 
@@ -394,6 +395,22 @@ class MessageTest extends MediaWikiLangTestCase {
                $this->assertSame( 'example &amp;', $msg->escaped() );
        }
 
+       public function testRawHtmlInMsg() {
+               global $wgParserConf;
+               $this->setMwGlobals( 'wgRawHtml', true );
+               // We have to reset the core hook registration.
+               // to register the html hook
+               MessageCache::destroyInstance();
+               $this->setMwGlobals( 'wgParser',
+                       ObjectFactory::constructClassInstance( $wgParserConf['class'], [ $wgParserConf ] )
+               );
+
+               $msg = new RawMessage( '<html><script>alert("xss")</script></html>' );
+               $txt = '<span class="error">&lt;html&gt; tags cannot be' .
+                       ' used outside of normal pages.</span>';
+               $this->assertSame( $txt, $msg->parse() );
+       }
+
        /**
         * @covers Message::params
         * @covers Message::toString