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