From c04ee360d9c221b3e6d7b71b3e51311e76389071 Mon Sep 17 00:00:00 2001
From: Max Semenik <maxsem.wiki@gmail.com>
Date: Mon, 9 May 2016 11:40:54 -0700
Subject: [PATCH] SECURITY: Fix XSS via __proto__

Bug: T134719
Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
---
 includes/SimpleStyleParser.php     | 17 +++++++++--------
 tests/phpunit/KartographerTest.php | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/includes/SimpleStyleParser.php b/includes/SimpleStyleParser.php
index 1157748..714ab27 100644
--- a/includes/SimpleStyleParser.php
+++ b/includes/SimpleStyleParser.php
@@ -14,8 +14,6 @@ use stdClass;
 class SimpleStyleParser {
 	private static $parsedProps = [ 'title', 'description' ];
 
-	private static $recursedProps = [ 'geometry', 'geometries', 'features' ];
-
 	/** @var Parser */
 	private $parser;
 
@@ -145,15 +143,18 @@ class SimpleStyleParser {
 			return;
 		}
 
-		if ( property_exists( $json, 'properties' ) && is_object( $json->properties ) ) {
-			$this->sanitizeProperties( $json->properties );
-		}
-
-		foreach ( self::$recursedProps as $prop ) {
-			if ( property_exists( $json, $prop ) ) {
+		foreach ( array_keys( get_object_vars( $json ) ) as $prop ) {
+			// https://phabricator.wikimedia.org/T134719
+			if ( $prop[0] === '_' ) {
+				unset( $json->$prop );
+			} else {
 				$this->sanitize( $json->$prop );
 			}
 		}
+
+		if ( property_exists( $json, 'properties' ) && is_object( $json->properties ) ) {
+			$this->sanitizeProperties( $json->properties );
+		}
 	}
 
 	/**
diff --git a/tests/phpunit/KartographerTest.php b/tests/phpunit/KartographerTest.php
index 4d5ad2e..ae27596 100644
--- a/tests/phpunit/KartographerTest.php
+++ b/tests/phpunit/KartographerTest.php
@@ -64,7 +64,36 @@ class KartographerTest extends MediaWikiTestCase {
       "marker-size": "medium",
       "marker-color": "0050d0"
     }
-  }';
+}';
+		$xssJson = '[
+  {
+	"__proto__": { "some": "bad stuff" },
+	"type": "Feature",
+	"geometry": {
+		"type": "Point",
+		"coordinates": [-122.3988, 37.8013]
+	},
+	"properties": {
+		"__proto__": { "foo": "bar" },
+		"title": "Foo bar"
+	}
+  },
+  {
+	"type": "GeometryCollection",
+	"geometries": [
+		{
+			"__proto__": "recurse me",
+			"type": "Point",
+			"coordinates": [ 0, 0 ],
+			"properties": { "__proto__": "is evil" }
+		}
+	]
+  }
+]';
+		$xssJsonSanitized = '{"_a4d5387a1b7974bf854321421a36d913101f5724":[
+			{"type":"Feature","geometry":{"type":"Point","coordinates":[-122.3988,37.8013]},"properties":{"title":"Foo bar"}},
+			{"type":"GeometryCollection","geometries":[{"type":"Point","coordinates":[0,0],"properties":{}}]}
+		]}';
 		$wikitextJsonParsed = '{"_be34df99c99d1efd9eaa8eabc87a43f2541a67e5":[
 				{"type":"Feature","geometry":{"type":"Point","coordinates":[-122.3988,37.8013]},
 				"properties":{"title":"&lt;script&gt;alert(document.cookie);&lt;\/script&gt;",
@@ -79,8 +108,10 @@ class KartographerTest extends MediaWikiTestCase {
 			[ "{\"_4622d19afa2e6480c327846395ed932ba6fa56d4\":[$validJson]}", "<mapframe width=700 height=400 zoom=13 longitude=-122.3988 latitude=37.8013>[$validJson]</mapframe>", '<mapframe> with GeoJSON array' ],
 			[ $wikitextJsonParsed, "<mapframe width=700 height=400 zoom=13 longitude=-122.3988 latitude=37.8013>[{$this->wikitextJson}]</mapframe>", '<mapframe> with parsable text and description' ],
 			[ $wikitextJsonParsed, "<maplink zoom=13 longitude=-122.3988 latitude=37.8013>[{$this->wikitextJson}]</maplink>", '<maplink> with parsable text and description' ],
+
 			// Bugs
 			[ 'null', "<maplink zoom=13 longitude=-122.3988 latitude=37.8013>\t\r\n </maplink>", 'T127345: whitespace-only tag content, <maplink>' ],
+			[ $xssJsonSanitized, "<maplink zoom=13 longitude=10 latitude=20>$xssJson</maplink>", 'T134719: XSS via __proto__' ],
 		];
 	}
 
-- 
2.7.2

