MaxSem has uploaded a new change for review.
https://gerrit.wikimedia.org/r/294373
Change subject: SECURITY: Fix XSS via __proto__
......................................................................
SECURITY: Fix XSS via __proto__
Bug: T134719
Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
---
M includes/SimpleStyleParser.php
M tests/phpunit/KartographerTest.php
2 files changed, 41 insertions(+), 9 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Kartographer
refs/changes/73/294373/1
diff --git a/includes/SimpleStyleParser.php b/includes/SimpleStyleParser.php
index 5acca96..48deb2e 100644
--- a/includes/SimpleStyleParser.php
+++ b/includes/SimpleStyleParser.php
@@ -17,8 +17,6 @@
class SimpleStyleParser {
private static $parsedProps = [ 'title', 'description' ];
- private static $recursedProps = [ 'geometry', 'geometries', 'features'
];
-
/** @var Parser */
private $parser;
@@ -153,15 +151,18 @@
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 65c858e..8995c7c 100644
--- a/tests/phpunit/KartographerTest.php
+++ b/tests/phpunit/KartographerTest.php
@@ -67,8 +67,37 @@
"marker-size": "medium",
"marker-color": "0050d0"
}
- }';
+ }';
/** @noinspection HtmlUnknownTarget */
+ $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":"<script>alert(document.cookie);<\/script>",
@@ -98,8 +127,10 @@
}</mapframe>', 'Invalid JSON 6' ],
[ $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__' ],
];
}
--
To view, visit https://gerrit.wikimedia.org/r/294373
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Kartographer
Gerrit-Branch: master
Gerrit-Owner: MaxSem <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
