Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
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
This commit is contained in:
Brian Wolff 2016-06-29 18:09:18 -04:00
parent 62638d7845
commit 184658eb32
4 changed files with 26 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
includes/Message.php
View file

@ -802,10 +802,13 @@ class Message implements MessageSpecifier, Serializable {
$string = $this->fetchMessage();
if ( $string === false ) {
if ( $this->format === 'plain' || $this->format === 'text' ) {
return '<' . $this->key . '>';
}
return '&lt;' . htmlspecialchars( $this->key ) . '&gt;';
// Err on the side of safety, ensure that the output
// is always html safe in the event the message key is
// missing, since in that case its highly likely the
// 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.

2
tests/parser/parserTests.txt
View file

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

18
tests/phpunit/includes/MessageTest.php
View file

@ -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' )->plain() );
$this->assertEquals( '<i<dont>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' )->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&lt;dont&gt;exist-evar⧽', wfMessage( 'i<dont>exist-evar' )->plain() );
$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', '&lt;i-dont-exist-evar&gt;', 'escaped' ],
[ 'i-dont-exist-evar', '⧼i-dont-exist-evar⧽' ],
[ '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⧽' ],
];
}

16
tests/phpunit/includes/StatusTest.php
View file

@ -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>",
];