Do not allow proto in Json [v2]
No One
Actions

Authored By

	Bawolff
	May 18 2016, 1:22 AM

Size

5 KB

Referenced Files

None

Subscribers

None

Do not allow proto in Json [v2]
View Options

	From 29d4dfd706d72dd89d83dd7eacc00b23e39c4e56 Mon Sep 17 00:00:00 2001
	From: Brian Wolff <bawolff+wn@gmail.com>
	Date: Fri, 13 May 2016 04:27:11 -0400
	Subject: [PATCH] SECURITY: Do not allow keys named __proto__ in Json output

	Browsers interpret that key name specially when you create just
	a plain javascript object. In order to prevent unexepcted behaviour
	that in certain cases may be exploitable, replace the key name
	with a dummy key named "__proto__ IS NOT ALLOWED".

	This should not affect legitimate users, as there is no good
	reason to use "__proto__" as a key name.

	Bug: T134719
	Change-Id: Ice111d53d9b3ad164fca3a662e37450e7a2b75d6
	---
	includes/json/FormatJson.php \| 44 +++++++++++++++++++
	tests/phpunit/includes/json/FormatJsonTest.php \| 58 ++++++++++++++++++++++++++
	2 files changed, 102 insertions(+)

	diff --git a/includes/json/FormatJson.php b/includes/json/FormatJson.php
	index 775ab43..fd26111 100644
	--- a/includes/json/FormatJson.php
	+++ b/includes/json/FormatJson.php
	@@ -20,6 +20,8 @@
	* @file
	*/

	+use MediaWiki\Logger\LoggerFactory;
	+
	/**
	* JSON formatter wrapper class
	*/
	@@ -48,6 +50,8 @@ class FormatJson {
	/**
	* Skip escaping as many characters as reasonably possible.
	*
	+ * @note This doesn't include PROTO_OK, since banning __proto__ is a different
	+ * sort of escaping from the character escapes.
	* @warning When generating inline script blocks, use FormatJson::UTF8_OK instead.
	*
	* @since 1.22
	@@ -55,6 +59,18 @@ class FormatJson {
	const ALL_OK = 3;

	/**
	+ * Allow __proto__ as a key name.
	+ *
	+ * Without this any key named __proto__ will be replaced with "__proto__ IS NOT ALLOWED"
	+ * @warning Do not use this if having the JSON directly interpreted as Javascript.
	+ * Web browsers treat __proto__ as containing the object's prototype, which could
	+ * potentially be used to trick sanitization filters.
	+ *
	+ * @since 1.28
	+ */
	+ const PROTO_OK = 4;
	+
	+ /**
	* If set, treat json objects '{...}' as associative arrays. Without this option,
	* json objects will be converted to stdClass.
	* The value is set to 1 to be backward compatible with 'true' that was used before.
	@@ -89,6 +105,13 @@ class FormatJson {
	const WS_CLEANUP_REGEX = '/(?<=[\[{])\n\s*+(?=[\]}])/';

	/**
	+ * Regex for detecting keys named __proto__
	+ *
	+ * @private
	+ */
	+ const PROTO_REGEX = '/(?<!\\\\)"__proto__":/';
	+
	+ /**
	* Characters problematic in JavaScript.
	*
	* @note These are listed in ECMA-262 (5.1 Ed.), §7.3 Line Terminators along with U+000A (LF)
	@@ -168,6 +191,27 @@ class FormatJson {
	$json = str_replace( self::$badChars, self::$badCharsEscaped, $json );
	}

	+ if ( !( $escaping & self::PROTO_OK ) ) {
	+ if ( preg_match( self::PROTO_REGEX, $json ) !== 0 ) {
	+ $json = preg_replace( self::PROTO_REGEX, '"__proto__ IS NOT ALLOWED":', $json );
	+ LoggerFactory::getInstance( 'json' )->warning(
	+ '{caller} tried to encode Json containing __proto__',
	+ [
	+ 'json' => $json,
	+ 'method' => __METHOD__,
	+ 'caller' => wfGetCaller()
	+ ]
	+ );
	+ // Some paranoia just in case. There are no circumstances that
	+ // should lead to this branch being reached.
	+ if ( preg_match( self::PROTO_REGEX, $json ) !== 0
	+ \|\| !FormatJson::parse( $json )->isGood()
	+ ) {
	+ throw new Exception( "Key named __proto__ in json" );
	+ }
	+ }
	+ }
	+
	return $json;
	}

	diff --git a/tests/phpunit/includes/json/FormatJsonTest.php b/tests/phpunit/includes/json/FormatJsonTest.php
	index 01b575c..b199b5b 100644
	--- a/tests/phpunit/includes/json/FormatJsonTest.php
	+++ b/tests/phpunit/includes/json/FormatJsonTest.php
	@@ -372,4 +372,62 @@ class FormatJsonTest extends MediaWikiTestCase {

	return $cases;
	}
	+
	+ public function testProtoOK() {
	+ $res = FormatJson::encode(
	+ [ '__proto__' => [ 'foo' => 'bar' ] ],
	+ false,
	+ FormatJson::PROTO_OK
	+ );
	+ $this->assertEquals( '{"__proto__":{"foo":"bar"}}', $res );
	+ }
	+
	+ /**
	+ * @dataProvider provideProtoNotOkMisses
	+ */
	+ public function testProtoNotOkMisses( $expectedUgly, $expectedPretty, $data ) {
	+ $actualUgly = FormatJson::encode( $data, false );
	+ $actualPretty = FormatJson::encode( $data, true );
	+
	+ $this->assertEquals( $expectedUgly, $actualUgly, "ugly" );
	+ $this->assertEquals( $expectedPretty, $actualPretty, "pretty" );
	+ }
	+
	+ public function provideProtoNotOkMisses() {
	+ return [
	+ [
	+ '{"\"__proto__":{}}',
	+ "{\n \"\\\"__proto__\": {}\n}",
	+ [ '"__proto__' => new stdClass ]
	+ ],
	+ [
	+ '{"__Proto__":{}}',
	+ "{\n \"__Proto__\": {}\n}",
	+ [ '__Proto__' => new stdClass ]
	+ ],
	+ [
	+ '{"__PROTO__":{}}',
	+ "{\n \"__PROTO__\": {}\n}",
	+ [ '__PROTO__' => new stdClass ]
	+ ],
	+ [
	+ '{"foo":"__proto__"}',
	+ "{\n \"foo\": \"__proto__\"\n}",
	+ [ 'foo' => "__proto__" ]
	+ ],
	+ ];
	+ }
	+
	+ public function testProtoNotOk() {
	+ $data = [ 'foo' => [ 'bar' => 3, '__proto__' => [ 'baz' => 2 ], 'fred' => 'f' ] ];
	+ $resUgly = FormatJson::encode( $data, false );
	+ // @codingStandardsIgnoreStart Generic.Files.LineLength
	+ $expectedUgly = '{"foo":{"bar":3,"__proto__ IS NOT ALLOWED":{"baz":2},"fred":"f"}}';
	+ $this->assertEquals( $expectedUgly, $resUgly, "ugly" );
	+
	+ $expectedPretty ="{\n \"foo\": {\n \"bar\": 3,\n \"__proto__ IS NOT ALLOWED\": {\n \"baz\": 2\n },\n \"fred\": \"f\"\n }\n}";
	+ // @codingStandardsIgnoreEnd
	+ $resPretty = FormatJson::encode( $data, true );
	+ $this->assertEquals( $expectedPretty, $resPretty, "pretty" );
	+ }
	}
	--
	2.0.1

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 3735047
Default Alt Text: Do not allow __proto__ in Json [v2] (5 KB)

Do not allow __proto__ in Json [v2]No OneActions

Do not allow __proto__ in Json [v2]View Options

File Metadata

Event Timeline

Do not allow proto in Json [v2]
No One
Actions

Do not allow proto in Json [v2]
View Options