Make non-existent messages be html safe regardless of output format

If you have a non-existent message in the output, chances are its
user-controlled. If the message has the ->plain() or ->text()
format, the output used to be not safe for html. Obviously people
should not be using those format types where html is being outputted,
but sometimes that happens. I think we should prioritize always being
safe over the fallback content not potentially being double escaped.

Additionally switch the enclosing brackets to be fancy unicode
characters, to sidestep the escaping issue on the enclosing brackets.

So previously, wfMessage( 'script>alert(1)</script' )->text() would
have outputted <script>alert(1)</script>. Now it outputs
⧼script&gt;alert(1)&lt;/script⧽. No sane message key will include
< or >, so this would really only come up if the user can control
the message key name.

This goes somewhat against T68199.

Change-Id: Ic8a60892b8e847e6021494c10968814aac391731

includes/Message.php

@@ -802,10 +802,13 @@ class Message implements MessageSpecifier, Serializable {
 		$string = $this->fetchMessage();
 
 		if ( $string === false ) {
-			if ( $this->format === 'plain' || $this->format === 'text' ) {
+			// Err on the side of safety, ensure that the output
-				return '<' . $this->key . '>';
+			// is always html safe in the event the message key is
-			}
+			// missing, since in that case its highly likely the
-			return '&lt;' . htmlspecialchars( $this->key ) . '&gt;';
+			// message key is user-controlled.
+			// '⧼' is used instead of '<' to side-step any
+			// double-escaping issues.
+			return '⧼' . htmlspecialchars( $this->key ) . '⧽';
 		}
 
 		# Replace $* with a list of parameters for &uselang=qqx.

tests/parser/parserTests.txt

@@ -11013,7 +11013,7 @@ int keyword - non-existing message
 !! wikitext
 {{int:var}}
 !! html
-<p>&lt;var&gt;
+<p>⧼var⧽
 </p>
 !! end
 

tests/phpunit/includes/MessageTest.php

@@ -223,13 +223,13 @@ class MessageTest extends MediaWikiLangTestCase {
 	 */
 	public function testToStringKey() {
 		$this->assertEquals( 'Main Page', wfMessage( 'mainpage' )->text() );
-		$this->assertEquals( '<i-dont-exist-evar>', wfMessage( 'i-dont-exist-evar' )->text() );
+		$this->assertEquals( '⧼i-dont-exist-evar⧽', wfMessage( 'i-dont-exist-evar' )->text() );
-		$this->assertEquals( '<i<dont>exist-evar>', wfMessage( 'i<dont>exist-evar' )->text() );
+		$this->assertEquals( '⧼i&lt;dont&gt;exist-evar⧽', wfMessage( 'i<dont>exist-evar' )->text() );
-		$this->assertEquals( '<i-dont-exist-evar>', wfMessage( 'i-dont-exist-evar' )->plain() );
+		$this->assertEquals( '⧼i-dont-exist-evar⧽', wfMessage( 'i-dont-exist-evar' )->plain() );
-		$this->assertEquals( '<i<dont>exist-evar>', wfMessage( 'i<dont>exist-evar' )->plain() );
+		$this->assertEquals( '⧼i&lt;dont&gt;exist-evar⧽', wfMessage( 'i<dont>exist-evar' )->plain() );
-		$this->assertEquals( '&lt;i-dont-exist-evar&gt;', wfMessage( 'i-dont-exist-evar' )->escaped() );
+		$this->assertEquals( '⧼i-dont-exist-evar⧽', wfMessage( 'i-dont-exist-evar' )->escaped() );
 		$this->assertEquals(
-			'&lt;i&lt;dont&gt;exist-evar&gt;',
+			'⧼i&lt;dont&gt;exist-evar⧽',
 			wfMessage( 'i<dont>exist-evar' )->escaped()
 		);
 	}
@@ -237,8 +237,10 @@ class MessageTest extends MediaWikiLangTestCase {
 	public static function provideToString() {
 		return [
 			[ 'mainpage', 'Main Page' ],
-			[ 'i-dont-exist-evar', '<i-dont-exist-evar>' ],
+			[ 'i-dont-exist-evar', '⧼i-dont-exist-evar⧽' ],
-			[ 'i-dont-exist-evar', '&lt;i-dont-exist-evar&gt;', 'escaped' ],
+			[ 'i-dont-exist-evar', '⧼i-dont-exist-evar⧽', 'escaped' ],
+			[ 'script>alert(1)</script', '⧼script&gt;alert(1)&lt;/script⧽', 'escaped' ],
+			[ 'script>alert(1)</script', '⧼script&gt;alert(1)&lt;/script⧽' ],
 		];
 	}
 

tests/phpunit/includes/StatusTest.php

@@ -376,9 +376,9 @@ class StatusTest extends MediaWikiLangTestCase {
 		$status->warning( 'fooBar!' );
 		$testCases['1StringWarning'] = [
 			$status,
-			"<fooBar!>",
+			"⧼fooBar!⧽",
 			"(wrap-short: (fooBar!))",
-			"<p>&lt;fooBar!&gt;\n</p>",
+			"<p>⧼fooBar!⧽\n</p>",
 			"<p>(wrap-short: (fooBar!))\n</p>",
 		];
 
@@ -387,9 +387,9 @@ class StatusTest extends MediaWikiLangTestCase {
 		$status->warning( 'fooBar2!' );
 		$testCases['2StringWarnings'] = [
 			$status,
-			"* <fooBar!>\n* <fooBar2!>\n",
+			"* ⧼fooBar!⧽\n* ⧼fooBar2!⧽\n",
 			"(wrap-long: * (fooBar!)\n* (fooBar2!)\n)",
-			"<ul><li> &lt;fooBar!&gt;</li>\n<li> &lt;fooBar2!&gt;</li></ul>\n",
+			"<ul><li> ⧼fooBar!⧽</li>\n<li> ⧼fooBar2!⧽</li></ul>\n",
 			"<p>(wrap-long: * (fooBar!)\n</p>\n<ul><li> (fooBar2!)</li></ul>\n<p>)\n</p>",
 		];
 
@@ -397,9 +397,9 @@ class StatusTest extends MediaWikiLangTestCase {
 		$status->warning( new Message( 'fooBar!', [ 'foo', 'bar' ] ) );
 		$testCases['1MessageWarning'] = [
 			$status,
-			"<fooBar!>",
+			"⧼fooBar!⧽",
 			"(wrap-short: (fooBar!: foo, bar))",
-			"<p>&lt;fooBar!&gt;\n</p>",
+			"<p>⧼fooBar!⧽\n</p>",
 			"<p>(wrap-short: (fooBar!: foo, bar))\n</p>",
 		];
 
@@ -408,9 +408,9 @@ class StatusTest extends MediaWikiLangTestCase {
 		$status->warning( new Message( 'fooBar2!' ) );
 		$testCases['2MessageWarnings'] = [
 			$status,
-			"* <fooBar!>\n* <fooBar2!>\n",
+			"* ⧼fooBar!⧽\n* ⧼fooBar2!⧽\n",
 			"(wrap-long: * (fooBar!: foo, bar)\n* (fooBar2!)\n)",
-			"<ul><li> &lt;fooBar!&gt;</li>\n<li> &lt;fooBar2!&gt;</li></ul>\n",
+			"<ul><li> ⧼fooBar!⧽</li>\n<li> ⧼fooBar2!⧽</li></ul>\n",
 			"<p>(wrap-long: * (fooBar!: foo, bar)\n</p>\n<ul><li> (fooBar2!)</li></ul>\n<p>)\n</p>",
 		];