Reedy has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/345978 )
Change subject: SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true; ...................................................................... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true; 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 --- M includes/OutputPage.php M includes/cache/MessageCache.php M includes/parser/CoreTagHooks.php M includes/parser/ParserOptions.php M languages/i18n/en.json M languages/i18n/qqq.json M tests/phpunit/includes/MessageTest.php 7 files changed, 77 insertions(+), 4 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/78/345978/1 diff --git a/includes/OutputPage.php b/includes/OutputPage.php index a8be748..40de17d 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -1515,6 +1515,7 @@ // 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; @@ -1528,6 +1529,7 @@ // either. $po = ParserOptions::newFromAnon(); $po->setEditSection( false ); + $po->setAllowUnsafeRawHtml( false ); $po->isBogus = true; if ( $options !== null ) { $this->mParserOptions = empty( $options->isBogus ) ? $options : null; @@ -1537,6 +1539,7 @@ $this->mParserOptions = ParserOptions::newFromContext( $this->getContext() ); $this->mParserOptions->setEditSection( false ); + $this->mParserOptions->setAllowUnsafeRawHtml( false ); } if ( $options !== null && !empty( $options->isBogus ) ) { diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php index dc8c589..d6a0e8a 100644 --- a/includes/cache/MessageCache.php +++ b/includes/cache/MessageCache.php @@ -179,11 +179,16 @@ // 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; diff --git a/includes/parser/CoreTagHooks.php b/includes/parser/CoreTagHooks.php index c943b7c..438603a 100644 --- a/includes/parser/CoreTagHooks.php +++ b/includes/parser/CoreTagHooks.php @@ -79,12 +79,25 @@ * @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' ); } diff --git a/includes/parser/ParserOptions.php b/includes/parser/ParserOptions.php index 9a10878..52c64f3 100644 --- a/includes/parser/ParserOptions.php +++ b/includes/parser/ParserOptions.php @@ -243,6 +243,21 @@ */ 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 @@ 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 ); } @@ -597,6 +621,15 @@ } /** + * @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. * * Note that setting or changing this does not *make* the page a redirect diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 5326203..fcdfc9a 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -4239,5 +4239,6 @@ "restrictionsfield-label": "Allowed IP ranges:", "restrictionsfield-help": "One IP address or CIDR range per line. To enable everything, use<br><code>0.0.0.0/0</code><br><code>::/0</code>", "edit-error-short": "Error: $1", - "edit-error-long": "Errors:\n\n$1" + "edit-error-long": "Errors:\n\n$1", + "rawhtml-notallowed": "<html> tags cannot be used outside of normal pages." } diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 87fc9eb..55fbf05 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -4423,5 +4423,6 @@ "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).", "edit-error-short": "Error message. Parameters:\n* $1 - the error details\nSee also:\n* {{msg-mw|edit-error-long}}\n{{Identical|Error}}", - "edit-error-long": "Error message. Parameters:\n* $1 - the error details\nSee also:\n* {{msg-mw|edit-error-short}}\n{{Identical|Error}}" + "edit-error-long": "Error message. Parameters:\n* $1 - the error details\nSee also:\n* {{msg-mw|edit-error-short}}\n{{Identical|Error}}", + "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an <html> tag in a system message or somewhere other than a normal page." } diff --git a/tests/phpunit/includes/MessageTest.php b/tests/phpunit/includes/MessageTest.php index 4c689ab..29de0ad 100644 --- a/tests/phpunit/includes/MessageTest.php +++ b/tests/phpunit/includes/MessageTest.php @@ -1,4 +1,5 @@ <?php +use MediaWiki\MediaWikiServices; class MessageTest extends MediaWikiLangTestCase { @@ -306,6 +307,22 @@ $this->assertEquals( 'example &', $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"><html> tags cannot be' . + ' used outside of normal pages.</span>'; + $this->assertSame( $txt, $msg->parse() ); + } + /** * @covers Message::params * @covers Message::toString -- To view, visit https://gerrit.wikimedia.org/r/345978 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I661f482986d319cf41da1d3e7b20a0f028a42e90 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: REL1_28 Gerrit-Owner: Reedy <re...@wikimedia.org> Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits