Page MenuHomePhabricator
Authored By
MaxSem
May 9 2016, 6:43 PM
Size
3 KB
Referenced Files
None
Subscribers
None

T134719.patch

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

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
3712863
Default Alt Text
T134719.patch (3 KB)

Event Timeline