Make rawHTML mode not apply to system messages
Closed, ResolvedPublic

Description

I would normally consider something like wfMessage( 'foo' )->params( $_GET['foo'] )->parse() to be safe. However in raw html mode it would not be. Perhaps as a hardening measure we should disable raw html on the message parser.

I took a look through core MW and did not find anything obvious of this form that was exploitable.

Bawolff created this task.Jan 24 2017, 9:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 24 2017, 9:06 PM
dpatrick triaged this task as Normal priority.Jan 24 2017, 9:45 PM

Going to send an email to wikitech-l to get more feedback on this.

I looked through donatewiki and foundationwiki. It does not appear that rawhtml is being used in system messages there.

Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Jan 31 2017, 8:56 PM
Tgr updated the task description. (Show Details)Jan 31 2017, 9:44 PM
Tgr added a subscriber: Tgr.Jan 31 2017, 9:47 PM

That would make it impossible to use HTML in the page footer (e.g. for social shares), which I would expect to be one of the more common use cases.

Having params sanitize it would be a more targeted measure (although probably harder to implement).

It's ok for the <html> tag to not work in namespace 8 even if $wgRawHtml is true. This is what you're proposing, right?

T45646/T2212 are another matter, aren't they.

It's ok for the <html> tag to not work in namespace 8 even if $wgRawHtml is true. This is what you're proposing, right?

It would still work for direct views to the MediaWiki namespace. It just wouldn't work when the message is processed through wfMessage() [e.g. When the message is included on special pages, etc]

T45646/T2212 are another matter, aren't they.

Yes. This proposal does not include messages that are treated entirely raw. It only affects the <html> parser tag.

That would make it impossible to use HTML in the page footer (e.g. for social shares), which I would expect to be one of the more common use cases.

Well in fairness, people can directly add that to MediaWiki:Copyright. In general though I would hope such people use an extension.

Having params sanitize it would be a more targeted measure (although probably harder to implement).

We could introduce a variant of params that basically uses wfEscapeWikiText(), however I think it would be very difficult to escape only <html> tags but not other wikisyntax since the html tags can come in via templates and things.

I support this and it nicely resonates with T2212: Some MediaWiki: messages not safe in HTML (tracking). Anything that requires unsafe output must find another way to do it.

Krinkle added a subscriber: Krinkle.Feb 2 2017, 2:52 AM

It's ok for the <html> tag to not work in namespace 8 even if $wgRawHtml is true. This is what you're proposing, right?

It would still work for direct views to the MediaWiki namespace. It just wouldn't work when the message is processed through wfMessage()

There's a few cases I know of that surprised me by being interface messages. Here's a few of them to remember that these would also be affected:

And various other *-summary and *text messages like these are essentially considered wiki pages (including interwiki/language links).

sp-contributions-footer

ironically, that one is actually pretty close to being an example of this issue. However, its not since < cannot be used in a title (or username).

So there's three main ways I see to implement this:

  1. Use the existing interface flag

Pros: Simple and easy to implement. Already there.
Cons: Would not prevent <html> when someone does ->inLanguage() or calls an OutputPage method (Should OutputPage addWikiText() stuff be included in this proposal?)

  1. Add a new option to ParserOptions

Pros: Would work everywhere we want it to
Cons: Add yet another option to ParserOptions

  1. Only register CoreTagHooks::html in the main parser, and not in the MessageCache parser (or try and unregister it in MessageCache)

This seems kind of hacky, especially given how the parser is often initialized once and then reused.

I think I like method 2 the best.

Tgr added a comment.Feb 4 2017, 10:49 PM

Pass some flag to the parser hooks which tells whether the main page content or a message is being parsed, and use that in CoreTagHooks::html? The usefulness of such a flag came up a number of times in the past.

jeremyb added a subscriber: jeremyb.Feb 6 2017, 3:41 AM

Change 336389 had a related patch set uploaded (by Brian Wolff):
SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/336389

Change 336389 had a related patch set uploaded (by Brian Wolff):
SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/336389

This is just meant as one possible implementation, not necessarily the one we will use. I wrote this patch before reading tgr's comment above.

To clarify, this will also disable <html> tags on stuff run through $wgOut->addWikiText() [and related methods], since that also has similar parameterization. This means of course that the change will affect more things such as special pages, including special pages that are transcluded on to normal pages.

Pass some flag to the parser hooks which tells whether the main page content or a message is being parsed, and use that in CoreTagHooks::html? The usefulness of such a flag came up a number of times in the past.

Hmm. That is kind of what $parser->getOptions()->getInterfaceMessage() in one sense, but then that flag does a whole lot of things, and sometimes gets turned off for various types of interface messages. I guess what people want to know, is this a normal wiki page parse, or is this something else like an interface message (but there are other things that are neither normal page parses nor interface messages).

The benefit of using a specialized flag just for this, is no one will mess with it to trigger some unrelated behaviour tied to the flag, that might happen if its a general "This is a normal wikipage" flag.

This also raises a question - should the "disable <html> thingies" be an on by default behaviour that gets disabled in the normal parser, or an off by default that gets enabled in system message contexts.

Change 336389 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/336389

Krinkle removed a subscriber: Krinkle.
Reedy added a subscriber: Reedy.Apr 1 2017, 4:03 PM

I guess this needs cherry picking to REL1_28, REL1_27 and REL1_23...

Change 345978 had a related patch set uploaded (by Reedy; owner: Brian Wolff):
[mediawiki/core@REL1_28] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/345978

Reedy assigned this task to Bawolff.Apr 1 2017, 4:13 PM

Change 345979 had a related patch set uploaded (by Reedy; owner: Brian Wolff):
[mediawiki/core@REL1_27] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/345979

Reedy added a comment.Apr 1 2017, 4:21 PM

REL1_23 isn't a nice cherry pick...

diff --cc includes/OutputPage.php
index b3e724a,1985ab4..0000000
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@@ -1415,11 -1536,45 +1415,42 @@@ class OutputPage extends ContextSource 
  	 * @return ParserOptions
  	 */
  	public function parserOptions( $options = null ) {
++<<<<<<< HEAD
 +		if ( !$this->mParserOptions ) {
++=======
+ 		if ( $options !== null && !empty( $options->isBogus ) ) {
+ 			// Someone is trying to set a bogus pre-$wgUser PO. Check if it has
+ 			// 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;
+ 			}
+ 		}
+ 
+ 		if ( !$this->mParserOptions ) {
+ 			if ( !$this->getContext()->getUser()->isSafeToLoad() ) {
+ 				// $wgUser isn't unstubbable yet, so don't try to get a
+ 				// ParserOptions for it. And don't cache this ParserOptions
+ 				// either.
+ 				$po = ParserOptions::newFromAnon();
+ 				$po->setEditSection( false );
+ 				$po->setAllowUnsafeRawHtml( false );
+ 				$po->isBogus = true;
+ 				if ( $options !== null ) {
+ 					$this->mParserOptions = empty( $options->isBogus ) ? $options : null;
+ 				}
+ 				return $po;
+ 			}
+ 
++>>>>>>> 51b871e... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
  			$this->mParserOptions = ParserOptions::newFromContext( $this->getContext() );
  			$this->mParserOptions->setEditSection( false );
+ 			$this->mParserOptions->setAllowUnsafeRawHtml( false );
  		}
 -
 -		if ( $options !== null && !empty( $options->isBogus ) ) {
 -			// They're trying to restore the bogus pre-$wgUser PO. Do the right
 -			// thing.
 -			return wfSetVar( $this->mParserOptions, null, true );
 -		} else {
 -			return wfSetVar( $this->mParserOptions, $options );
 -		}
 +		return wfSetVar( $this->mParserOptions, $options );
  	}
  
  	/**
diff --cc includes/cache/MessageCache.php
index daaa915,3910bd3..0000000
--- a/includes/cache/MessageCache.php
+++ b/includes/cache/MessageCache.php
@@@ -144,9 -168,25 +144,26 @@@ class MessageCache 
  	 * @return ParserOptions
  	 */
  	function getParserOptions() {
 -		global $wgUser;
 -
  		if ( !$this->mParserOptions ) {
++<<<<<<< HEAD
++=======
+ 			if ( !$wgUser->isSafeToLoad() ) {
+ 				// $wgUser isn't unstubbable yet, so don't try to get a
+ 				// ParserOptions for it. And don't cache this ParserOptions
+ 				// either.
+ 				$po = ParserOptions::newFromAnon();
+ 				$po->setEditSection( false );
+ 				$po->setAllowUnsafeRawHtml( false );
+ 				return $po;
+ 			}
+ 
++>>>>>>> 51b871e... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
  			$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 --cc includes/parser/CoreTagHooks.php
index 71f3faa,438603a..0000000
--- a/includes/parser/CoreTagHooks.php
+++ b/includes/parser/CoreTagHooks.php
@@@ -74,16 -75,29 +74,33 @@@ class CoreTagHooks 
  	 *
  	 * Uses undocumented extended tag hook return values, introduced in r61913.
  	 *
 -	 * @param string $content
 -	 * @param array $attributes
 -	 * @param Parser $parser
 +	 * @param $content string
 +	 * @param $attributes array
 +	 * @param $parser Parser
  	 * @throws MWException
- 	 * @return array
+ 	 * @return array|string Output of tag hook
  	 */
 -	public static function html( $content, $attributes, $parser ) {
 +	static function html( $content, $attributes, $parser ) {
  		global $wgRawHtml;
  		if ( $wgRawHtml ) {
++<<<<<<< HEAD
 +			return array( $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()
+ 				);
+ 			}
++>>>>>>> 51b871e... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
  		} else {
  			throw new MWException( '<html> extension tag encountered unexpectedly' );
  		}
diff --cc includes/parser/ParserOptions.php
index edd4911,7946c89..0000000
--- a/includes/parser/ParserOptions.php
+++ b/includes/parser/ParserOptions.php
@@@ -208,46 -213,165 +208,135 @@@ class ParserOptions 
  	/**
  	 * Function to be called when an option is accessed.
  	 */
++<<<<<<< HEAD
 +	protected $onAccessCallback = null;
 +
 +	function getInterwikiMagic()                { return $this->mInterwikiMagic; }
 +	function getAllowExternalImages()           { return $this->mAllowExternalImages; }
 +	function getAllowExternalImagesFrom()       { return $this->mAllowExternalImagesFrom; }
 +	function getEnableImageWhitelist()          { return $this->mEnableImageWhitelist; }
 +	function getEditSection()                   { return $this->mEditSection; }
 +	function getNumberHeadings()                { $this->optionUsed( 'numberheadings' );
 +												  return $this->mNumberHeadings; }
 +	function getAllowSpecialInclusion()         { return $this->mAllowSpecialInclusion; }
 +	function getTidy()                          { return $this->mTidy; }
 +	function getInterfaceMessage()              { return $this->mInterfaceMessage; }
 +	function getTargetLanguage()                { return $this->mTargetLanguage; }
 +	function getMaxIncludeSize()                { return $this->mMaxIncludeSize; }
 +	function getMaxPPNodeCount()                { return $this->mMaxPPNodeCount; }
 +	function getMaxGeneratedPPNodeCount()       { return $this->mMaxGeneratedPPNodeCount; }
 +	function getMaxPPExpandDepth()              { return $this->mMaxPPExpandDepth; }
 +	function getMaxTemplateDepth()              { return $this->mMaxTemplateDepth; }
++=======
+ 	private $onAccessCallback = null;
+ 
+ 	/**
+ 	 * If the page being parsed is a redirect, this should hold the redirect
+ 	 * target.
+ 	 * @var Title|null
+ 	 */
+ 	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;
+ 	}
+ 
+ 	public function getAllowExternalImages() {
+ 		return $this->mAllowExternalImages;
+ 	}
+ 
+ 	public function getAllowExternalImagesFrom() {
+ 		return $this->mAllowExternalImagesFrom;
+ 	}
+ 
+ 	public function getEnableImageWhitelist() {
+ 		return $this->mEnableImageWhitelist;
+ 	}
+ 
+ 	public function getEditSection() {
+ 		return $this->mEditSection;
+ 	}
+ 
+ 	public function getNumberHeadings() {
+ 		$this->optionUsed( 'numberheadings' );
+ 
+ 		return $this->mNumberHeadings;
+ 	}
+ 
+ 	public function getAllowSpecialInclusion() {
+ 		return $this->mAllowSpecialInclusion;
+ 	}
+ 
+ 	public function getTidy() {
+ 		return $this->mTidy;
+ 	}
+ 
+ 	public function getInterfaceMessage() {
+ 		return $this->mInterfaceMessage;
+ 	}
+ 
+ 	public function getTargetLanguage() {
+ 		return $this->mTargetLanguage;
+ 	}
+ 
+ 	public function getMaxIncludeSize() {
+ 		return $this->mMaxIncludeSize;
+ 	}
+ 
+ 	public function getMaxPPNodeCount() {
+ 		return $this->mMaxPPNodeCount;
+ 	}
+ 
+ 	public function getMaxGeneratedPPNodeCount() {
+ 		return $this->mMaxGeneratedPPNodeCount;
+ 	}
+ 
+ 	public function getMaxPPExpandDepth() {
+ 		return $this->mMaxPPExpandDepth;
+ 	}
+ 
+ 	public function getMaxTemplateDepth() {
+ 		return $this->mMaxTemplateDepth;
+ 	}
+ 
++>>>>>>> 51b871e... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
  	/* @since 1.20 */
 -	public function getExpensiveParserFunctionLimit() {
 -		return $this->mExpensiveParserFunctionLimit;
 -	}
 -
 -	public function getRemoveComments() {
 -		return $this->mRemoveComments;
 -	}
 -
 -	/* @since 1.24 */
 -	public function getCurrentRevisionCallback() {
 -		return $this->mCurrentRevisionCallback;
 -	}
 -
 -	public function getTemplateCallback() {
 -		return $this->mTemplateCallback;
 -	}
 -
 -	public function getEnableLimitReport() {
 -		return $this->mEnableLimitReport;
 -	}
 -
 -	public function getCleanSignatures() {
 -		return $this->mCleanSignatures;
 -	}
 -
 -	public function getExternalLinkTarget() {
 -		return $this->mExternalLinkTarget;
 -	}
 -
 -	public function getDisableContentConversion() {
 -		return $this->mDisableContentConversion;
 -	}
 -
 -	public function getDisableTitleConversion() {
 -		return $this->mDisableTitleConversion;
 -	}
 -
 -	public function getThumbSize() {
 -		$this->optionUsed( 'thumbsize' );
 -
 -		return $this->mThumbSize;
 -	}
 -
 -	public function getStubThreshold() {
 -		$this->optionUsed( 'stubthreshold' );
 -
 -		return $this->mStubThreshold;
 -	}
 -
 -	public function getIsPreview() {
 -		return $this->mIsPreview;
 -	}
 -
 -	public function getIsSectionPreview() {
 -		return $this->mIsSectionPreview;
 -	}
 -
 -	public function getIsPrintable() {
 -		$this->optionUsed( 'printable' );
 -
 -		return $this->mIsPrintable;
 -	}
 -
 -	public function getUser() {
 -		return $this->mUser;
 -	}
 -
 -	public function getPreSaveTransform() {
 -		return $this->mPreSaveTransform;
 -	}
 -
 -	public function getDateFormat() {
 +	function getExpensiveParserFunctionLimit()  { return $this->mExpensiveParserFunctionLimit; }
 +	function getRemoveComments()                { return $this->mRemoveComments; }
 +	function getTemplateCallback()              { return $this->mTemplateCallback; }
 +	function getEnableLimitReport()             { return $this->mEnableLimitReport; }
 +	function getCleanSignatures()               { return $this->mCleanSignatures; }
 +	function getExternalLinkTarget()            { return $this->mExternalLinkTarget; }
 +	function getDisableContentConversion()      { return $this->mDisableContentConversion; }
 +	function getDisableTitleConversion()        { return $this->mDisableTitleConversion; }
 +	function getThumbSize()                     { $this->optionUsed( 'thumbsize' );
 +												  return $this->mThumbSize; }
 +	function getStubThreshold()                 { $this->optionUsed( 'stubthreshold' );
 +												  return $this->mStubThreshold; }
 +
 +	function getIsPreview()                     { return $this->mIsPreview; }
 +	function getIsSectionPreview()              { return $this->mIsSectionPreview; }
 +	function getIsPrintable()                   { $this->optionUsed( 'printable' );
 +												  return $this->mIsPrintable; }
 +	function getUser()                          { return $this->mUser; }
 +	function getPreSaveTransform()              { return $this->mPreSaveTransform; }
 +
 +	function getDateFormat() {
  		$this->optionUsed( 'dateformat' );
  		if ( !isset( $this->mDateFormat ) ) {
  			$this->mDateFormat = $this->mUser->getDatePreference();
@@@ -293,47 -424,180 +382,177 @@@
  		return $this->getUserLangObj()->getCode();
  	}
  
++<<<<<<< HEAD
 +	function setInterwikiMagic( $x )            { return wfSetVar( $this->mInterwikiMagic, $x ); }
 +	function setAllowExternalImages( $x )       { return wfSetVar( $this->mAllowExternalImages, $x ); }
 +	function setAllowExternalImagesFrom( $x )   { return wfSetVar( $this->mAllowExternalImagesFrom, $x ); }
 +	function setEnableImageWhitelist( $x )      { return wfSetVar( $this->mEnableImageWhitelist, $x ); }
 +	function setDateFormat( $x )                { return wfSetVar( $this->mDateFormat, $x ); }
 +	function setEditSection( $x )               { return wfSetVar( $this->mEditSection, $x ); }
 +	function setNumberHeadings( $x )            { return wfSetVar( $this->mNumberHeadings, $x ); }
 +	function setAllowSpecialInclusion( $x )     { return wfSetVar( $this->mAllowSpecialInclusion, $x ); }
 +	function setTidy( $x )                      { return wfSetVar( $this->mTidy, $x ); }
 +
 +	/** @deprecated in 1.19 */
 +	function setSkin( $x )                      { wfDeprecated( __METHOD__, '1.19' ); }
 +	function setInterfaceMessage( $x )          { return wfSetVar( $this->mInterfaceMessage, $x ); }
 +	function setTargetLanguage( $x )            { return wfSetVar( $this->mTargetLanguage, $x, true ); }
 +	function setMaxIncludeSize( $x )            { return wfSetVar( $this->mMaxIncludeSize, $x ); }
 +	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
+ 	 * @return bool
+ 	 */
+ 	public function getAllowUnsafeRawHtml() {
+ 		return $this->allowUnsafeRawHtml;
+ 	}
+ 
+ 	public function setInterwikiMagic( $x ) {
+ 		return wfSetVar( $this->mInterwikiMagic, $x );
+ 	}
+ 
+ 	public function setAllowExternalImages( $x ) {
+ 		return wfSetVar( $this->mAllowExternalImages, $x );
+ 	}
+ 
+ 	public function setAllowExternalImagesFrom( $x ) {
+ 		return wfSetVar( $this->mAllowExternalImagesFrom, $x );
+ 	}
+ 
+ 	public function setEnableImageWhitelist( $x ) {
+ 		return wfSetVar( $this->mEnableImageWhitelist, $x );
+ 	}
+ 
+ 	public function setDateFormat( $x ) {
+ 		return wfSetVar( $this->mDateFormat, $x );
+ 	}
+ 
+ 	public function setEditSection( $x ) {
+ 		return wfSetVar( $this->mEditSection, $x );
+ 	}
+ 
+ 	public function setNumberHeadings( $x ) {
+ 		return wfSetVar( $this->mNumberHeadings, $x );
+ 	}
+ 
+ 	public function setAllowSpecialInclusion( $x ) {
+ 		return wfSetVar( $this->mAllowSpecialInclusion, $x );
+ 	}
+ 
+ 	public function setTidy( $x ) {
+ 		return wfSetVar( $this->mTidy, $x );
+ 	}
+ 
+ 	public function setInterfaceMessage( $x ) {
+ 		return wfSetVar( $this->mInterfaceMessage, $x );
+ 	}
+ 
+ 	public function setTargetLanguage( $x ) {
+ 		return wfSetVar( $this->mTargetLanguage, $x, true );
+ 	}
+ 
+ 	public function setMaxIncludeSize( $x ) {
+ 		return wfSetVar( $this->mMaxIncludeSize, $x );
+ 	}
+ 
+ 	public function setMaxPPNodeCount( $x ) {
+ 		return wfSetVar( $this->mMaxPPNodeCount, $x );
+ 	}
+ 
+ 	public function setMaxGeneratedPPNodeCount( $x ) {
+ 		return wfSetVar( $this->mMaxGeneratedPPNodeCount, $x );
+ 	}
+ 
+ 	public function setMaxTemplateDepth( $x ) {
+ 		return wfSetVar( $this->mMaxTemplateDepth, $x );
+ 	}
+ 
++>>>>>>> 51b871e... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
  	/* @since 1.20 */
 -	public function setExpensiveParserFunctionLimit( $x ) {
 -		return wfSetVar( $this->mExpensiveParserFunctionLimit, $x );
 -	}
 -
 -	public function setRemoveComments( $x ) {
 -		return wfSetVar( $this->mRemoveComments, $x );
 -	}
 -
 -	/* @since 1.24 */
 -	public function setCurrentRevisionCallback( $x ) {
 -		return wfSetVar( $this->mCurrentRevisionCallback, $x );
 -	}
 -
 -	public function setTemplateCallback( $x ) {
 -		return wfSetVar( $this->mTemplateCallback, $x );
 -	}
 -
 -	public function enableLimitReport( $x = true ) {
 -		return wfSetVar( $this->mEnableLimitReport, $x );
 -	}
 -
 -	public function setTimestamp( $x ) {
 -		return wfSetVar( $this->mTimestamp, $x );
 -	}
 -
 -	public function setCleanSignatures( $x ) {
 -		return wfSetVar( $this->mCleanSignatures, $x );
 -	}
 -
 -	public function setExternalLinkTarget( $x ) {
 -		return wfSetVar( $this->mExternalLinkTarget, $x );
 -	}
 -
 -	public function disableContentConversion( $x = true ) {
 -		return wfSetVar( $this->mDisableContentConversion, $x );
 -	}
 -
 -	public function disableTitleConversion( $x = true ) {
 -		return wfSetVar( $this->mDisableTitleConversion, $x );
 -	}
 -
 -	public function setUserLang( $x ) {
 +	function setExpensiveParserFunctionLimit( $x ) { return wfSetVar( $this->mExpensiveParserFunctionLimit, $x ); }
 +	function setRemoveComments( $x )            { return wfSetVar( $this->mRemoveComments, $x ); }
 +	function setTemplateCallback( $x )          { return wfSetVar( $this->mTemplateCallback, $x ); }
 +	function enableLimitReport( $x = true )     { return wfSetVar( $this->mEnableLimitReport, $x ); }
 +	function setTimestamp( $x )                 { return wfSetVar( $this->mTimestamp, $x ); }
 +	function setCleanSignatures( $x )           { return wfSetVar( $this->mCleanSignatures, $x ); }
 +	function setExternalLinkTarget( $x )        { return wfSetVar( $this->mExternalLinkTarget, $x ); }
 +	function disableContentConversion( $x = true ) { return wfSetVar( $this->mDisableContentConversion, $x ); }
 +	function disableTitleConversion( $x = true ) { return wfSetVar( $this->mDisableTitleConversion, $x ); }
 +	function setUserLang( $x )                  {
  		if ( is_string( $x ) ) {
  			$x = Language::factory( $x );
  		}
 -
  		return wfSetVar( $this->mUserLang, $x );
  	}
 +	function setThumbSize( $x )                 { return wfSetVar( $this->mThumbSize, $x ); }
 +	function setStubThreshold( $x )             { return wfSetVar( $this->mStubThreshold, $x ); }
 +	function setPreSaveTransform( $x )          { return wfSetVar( $this->mPreSaveTransform, $x ); }
  
++<<<<<<< HEAD
 +	function setIsPreview( $x )                 { return wfSetVar( $this->mIsPreview, $x ); }
 +	function setIsSectionPreview( $x )          { return wfSetVar( $this->mIsSectionPreview, $x ); }
 +	function setIsPrintable( $x )               { return wfSetVar( $this->mIsPrintable, $x ); }
++=======
+ 	public function setThumbSize( $x ) {
+ 		return wfSetVar( $this->mThumbSize, $x );
+ 	}
+ 
+ 	public function setStubThreshold( $x ) {
+ 		return wfSetVar( $this->mStubThreshold, $x );
+ 	}
+ 
+ 	public function setPreSaveTransform( $x ) {
+ 		return wfSetVar( $this->mPreSaveTransform, $x );
+ 	}
+ 
+ 	public function setIsPreview( $x ) {
+ 		return wfSetVar( $this->mIsPreview, $x );
+ 	}
+ 
+ 	public function setIsSectionPreview( $x ) {
+ 		return wfSetVar( $this->mIsSectionPreview, $x );
+ 	}
+ 
+ 	public 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
+ 	 */
+ 	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
+ 	 * or change its target, it merely records the information for reference
+ 	 * during the parse.
+ 	 *
+ 	 * @since 1.24
+ 	 * @param Title|null $title
+ 	 */
+ 	function setRedirectTarget( $title ) {
+ 		$this->redirectTarget = $title;
+ 	}
+ 
+ 	/**
+ 	 * Get the previously-set redirect target.
+ 	 *
+ 	 * @since 1.24
+ 	 * @return Title|null
+ 	 */
+ 	function getRedirectTarget() {
+ 		return $this->redirectTarget;
+ 	}
++>>>>>>> 51b871e... SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;
  
  	/**
  	 * Extra key that should be present in the parser cache key.
Reedy renamed this task from Consider making rawHTML mode not apply to system messages to Make rawHTML mode not apply to system messages.Apr 1 2017, 4:24 PM

Change 346025 had a related patch set uploaded (by Brian Wolff):
[mediawiki/core@REL1_23] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/346025

Change 346025 merged by Brian Wolff:
[mediawiki/core@REL1_23] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/346025

Change 345979 merged by jenkins-bot:
[mediawiki/core@REL1_27] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/345979

Change 345978 merged by jenkins-bot:
[mediawiki/core@REL1_28] SECURITY: Disable <html> tag on system messages despite $wgRawHtml = true;

https://gerrit.wikimedia.org/r/345978

Bawolff closed this task as Resolved.Apr 1 2017, 10:46 PM

Change 346843 had a related patch set uploaded (by Chad; owner: Brian Wolff):
[mediawiki/core@master] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/346843

Change 346852 had a related patch set uploaded (by Chad; owner: Brian Wolff):
[mediawiki/core@REL1_27] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/346852

Change 346862 had a related patch set uploaded (by Chad; owner: Brian Wolff):
[mediawiki/core@REL1_28] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/346862

Change 346843 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/346843

Change 346862 merged by jenkins-bot:
[mediawiki/core@REL1_28] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/346862

Change 346852 merged by jenkins-bot:
[mediawiki/core@REL1_27] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/346852

Change 347039 had a related patch set uploaded (by Chad; owner: Brian Wolff):
[mediawiki/core@wmf/1.29.0-wmf.19] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/347039

Change 347039 merged by jenkins-bot:
[mediawiki/core@wmf/1.29.0-wmf.19] SECURITY: Escape wikitext content model/format in message

https://gerrit.wikimedia.org/r/347039

awight added a subscriber: awight.Apr 11 2017, 6:03 PM

See https://phabricator.wikimedia.org/T162716

This fix was a good idea, but we're currently facing a ton of breakage on donatewiki (34 blocked items on the main landing page) . You can take a look on mwdebug1002, at the moment.

We need a workaround--is there an alternative to addWikiText which will allow raw HTML and disallow URL parameters?

Change 347660 had a related patch set uploaded (by Awight):
[mediawiki/extensions/FundraiserLandingPage@master] Kludge an exception to allow raw HTML in system messages

https://gerrit.wikimedia.org/r/347660

Tgr added a comment.Apr 12 2017, 7:43 AM

Broke zerowiki as well, see T162771.

Tgr added a comment.Apr 12 2017, 7:50 AM

With the benefit of hindsight: this change should have included some logging so it is easy to spot when a given wiki is affected.

One way to reduce the impact is to only isable <html> in messages which have parameters. Seems kind of painful to pass that information around, though.

In the longer term, it would be nice to come up with some system for message metadata so messages can be whitelisted. (There would be many other security benefits, such as locking messages to the right parsing type.) Sounds like an MCR thing...

Change 347660 merged by jenkins-bot:
[mediawiki/extensions/FundraiserLandingPage@master] Kludge an exception to allow raw HTML in system messages

https://gerrit.wikimedia.org/r/347660

Change 347864 had a related patch set uploaded (by Awight):
[mediawiki/extensions/FundraiserLandingPage@wmf/1.29.0-wmf.19] Kludge an exception to allow raw HTML in system messages

https://gerrit.wikimedia.org/r/347864

Change 347866 had a related patch set uploaded (by Awight):
[mediawiki/extensions/FundraiserLandingPage@wmf/1.29.0-wmf.20] Kludge an exception to allow raw HTML in system messages

https://gerrit.wikimedia.org/r/347866

Change 347864 merged by jenkins-bot:
[mediawiki/extensions/FundraiserLandingPage@wmf/1.29.0-wmf.19] Kludge an exception to allow raw HTML in system messages

https://gerrit.wikimedia.org/r/347864

Change 347866 merged by jenkins-bot:
[mediawiki/extensions/FundraiserLandingPage@wmf/1.29.0-wmf.20] Kludge an exception to allow raw HTML in system messages

https://gerrit.wikimedia.org/r/347866