jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/336389 )
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(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 9ecfa23..d3e1373 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -1568,6 +1568,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;
@@ -1581,6 +1582,7 @@
// 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 @@
$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 7cd489a..70e1d9a 100644
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@ -191,11 +191,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 7be8281..2cdd8c7 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 0c8bf21..f9fbfd0 100644
--- a/languages/i18n/en.json
+++ b/languages/i18n/en.json
@@ -4283,5 +4283,6 @@
"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": "<html> tags cannot be used outside of
normal pages."
}
diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json
index 2951a4c..75be7aa 100644
--- a/languages/i18n/qqq.json
+++ b/languages/i18n/qqq.json
@@ -4470,5 +4470,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).",
"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 <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 424218e..58087c1 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 {
@@ -394,6 +395,22 @@
$this->assertSame( '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/336389
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I661f482986d319cf41da1d3e7b20a0f028a42e90
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: C. Scott Ananian <canan...@wikimedia.org>
Gerrit-Reviewer: Dpatrick <dpatr...@wikimedia.org>
Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: TTO <at.li...@live.com.au>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
