Brian Wolff has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/346025 )
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 RELEASE-NOTES-1.23 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 8 files changed, 75 insertions(+), 4 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/25/346025/1 diff --git a/RELEASE-NOTES-1.23 b/RELEASE-NOTES-1.23 index 4e1116a..1e5ba7e 100644 --- a/RELEASE-NOTES-1.23 +++ b/RELEASE-NOTES-1.23 @@ -7,6 +7,7 @@ === Changes since 1.23.15 === * (T68404) CSS3 attr() function with url type is no longer allowed in inline styles. +* (T156184) $wgRawHtml will no longer apply to internationalization messages. == MediaWiki 1.23.15 == diff --git a/includes/OutputPage.php b/includes/OutputPage.php index b3e724a..e978d6b 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -1418,6 +1418,7 @@ if ( !$this->mParserOptions ) { $this->mParserOptions = ParserOptions::newFromContext( $this->getContext() ); $this->mParserOptions->setEditSection( false ); + $this->mParserOptions->setAllowUnsafeRawHtml( false ); } return wfSetVar( $this->mParserOptions, $options ); } diff --git a/includes/cache/MessageCache.php b/includes/cache/MessageCache.php index daaa915..49e665c 100644 --- a/includes/cache/MessageCache.php +++ b/includes/cache/MessageCache.php @@ -147,6 +147,10 @@ if ( !$this->mParserOptions ) { $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 71f3faa..1f2ea82 100644 --- a/includes/parser/CoreTagHooks.php +++ b/includes/parser/CoreTagHooks.php @@ -78,12 +78,25 @@ * @param $attributes array * @param $parser Parser * @throws MWException - * @return array + * @return array|string Output of tag hook */ static function html( $content, $attributes, $parser ) { global $wgRawHtml; if ( $wgRawHtml ) { - return array( $content, 'markerType' => 'nowiki' ); + if ( $parser->getOptions()->getAllowUnsafeRawHtml() ) { + return array( $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', + array( '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 edd4911..9039203 100644 --- a/includes/parser/ParserOptions.php +++ b/includes/parser/ParserOptions.php @@ -226,6 +226,22 @@ function getMaxGeneratedPPNodeCount() { return $this->mMaxGeneratedPPNodeCount; } function getMaxPPExpandDepth() { return $this->mMaxPPExpandDepth; } function getMaxTemplateDepth() { return $this->mMaxTemplateDepth; } + + /** + * 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; + /* @since 1.20 */ function getExpensiveParserFunctionLimit() { return $this->mExpensiveParserFunctionLimit; } function getRemoveComments() { return $this->mRemoveComments; } @@ -311,6 +327,15 @@ function setMaxPPNodeCount( $x ) { return wfSetVar( $this->mMaxPPNodeCount, $x ); } function setMaxGeneratedPPNodeCount( $x ) { return wfSetVar( $this->mMaxGeneratedPPNodeCount, $x ); } function setMaxTemplateDepth( $x ) { return wfSetVar( $this->mMaxTemplateDepth, $x ); } + + /** + * @since 1.29 1.23.16 + * @return bool + */ + public function getAllowUnsafeRawHtml() { + return $this->allowUnsafeRawHtml; + } + /* @since 1.20 */ function setExpensiveParserFunctionLimit( $x ) { return wfSetVar( $this->mExpensiveParserFunctionLimit, $x ); } function setRemoveComments( $x ) { return wfSetVar( $this->mRemoveComments, $x ); } @@ -336,6 +361,15 @@ function setIsPrintable( $x ) { 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 and 1.23.16 + */ + public function setAllowUnsafeRawHtml( $x ) { + return wfSetVar( $this->allowUnsafeRawHtml, $x ); + } + + /** * Extra key that should be present in the parser cache key. */ function addExtraKey( $key ) { diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 8b674fa..6536928 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -3535,5 +3535,6 @@ "expand_templates_generate_rawhtml": "Show raw HTML", "expand_templates_preview": "Preview", "expand_templates_preview_fail_html": "<em>Because {{SITENAME}} has raw HTML enabled and there was a loss of session data, the preview is hidden as a precaution against JavaScript attacks.</em>\n\n<strong>If this is a legitimate preview attempt, please try again.</strong>\nIf it still does not work, try [[Special:UserLogout|logging out]] and logging back in.", - "expand_templates_preview_fail_html_anon": "<em>Because {{SITENAME}} has raw HTML enabled and you are not logged in, the preview is hidden as a precaution against JavaScript attacks.</em>\n\n<strong>If this is a legitimate preview attempt, please [[Special:UserLogin|log in]] and try again.</strong>" + "expand_templates_preview_fail_html_anon": "<em>Because {{SITENAME}} has raw HTML enabled and you are not logged in, the preview is hidden as a precaution against JavaScript attacks.</em>\n\n<strong>If this is a legitimate preview attempt, please [[Special:UserLogin|log in]] and try again.</strong>", + "rawhtml-notallowed": "<html> tags cannot be used outside of normal pages." } diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 4d8fb9b..c48f913 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -3699,5 +3699,6 @@ "expand_templates_generate_rawhtml": "Used as checkbox label.", "expand_templates_preview": "{{Identical|Preview}}", "expand_templates_preview_fail_html": "Used as error message in Preview section of [[Special:ExpandTemplates]] page.", - "expand_templates_preview_fail_html_anon": "Used as error message in Preview section of [[Special:ExpandTemplates]] page." + "expand_templates_preview_fail_html_anon": "Used as error message in Preview section of [[Special:ExpandTemplates]] page.", + "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 44ca3d2..5895581 100644 --- a/tests/phpunit/includes/MessageTest.php +++ b/tests/phpunit/includes/MessageTest.php @@ -145,6 +145,22 @@ $this->assertEquals( '(Заглавная страница $1)', wfMessage( 'parentheses' )->rawParams( 'Заглавная страница $1' )->plain() ); } + 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'], array( $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::__construct * @covers Message::params -- To view, visit https://gerrit.wikimedia.org/r/346025 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_23 Gerrit-Owner: Brian Wolff <bawolff...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits