[MediaWiki-commits] [Gerrit] SECURITY: Fix XSS via __proto__ - change (mediawiki...Kartographer)

2016-06-15 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

Change subject: SECURITY: Fix XSS via __proto__
..


SECURITY: Fix XSS via __proto__

Bug: T134719
Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
(cherry picked from commit 78bec3f4ecbd877719ad5fbcc8e6c0b4c19035ce)
---
M includes/SimpleStyleParser.php
M tests/phpunit/KartographerTest.php
2 files changed, 41 insertions(+), 9 deletions(-)

Approvals:
  Mattflaschen: Looks good to me, approved
  jenkins-bot: Verified



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":"scriptalert(document.cookie);\/script",
@@ -98,8 +127,10 @@
   }', 'Invalid JSON 6' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", ' with parsable 
text and description' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", 
' with parsable text and description' ],
+
// Bugs
[ 'null', "\t\r\n ", 'T127345: whitespace-only tag content, 
' ],
+   [ $xssJsonSanitized, "$xssJson", 'T134719: XSS via __proto__' ],
];
}
 

-- 
To view, visit https://gerrit.wikimedia.org/r/294650
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Kartographer
Gerrit-Branch: wmf/1.28.0-wmf.6
Gerrit-Owner: Mattflaschen 
Gerrit-Reviewer: Mattflaschen 
Gerrit-Reviewer: MaxSem 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] SECURITY: Fix XSS via __proto__ - change (mediawiki...Kartographer)

2016-06-15 Thread Mattflaschen (Code Review) 
Mattflaschen has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/294650

Change subject: SECURITY: Fix XSS via __proto__
..

SECURITY: Fix XSS via __proto__

Bug: T134719
Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
(cherry picked from commit 78bec3f4ecbd877719ad5fbcc8e6c0b4c19035ce)
---
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/50/294650/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":"scriptalert(document.cookie);\/script",
@@ -98,8 +127,10 @@
   }', 'Invalid JSON 6' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", ' with parsable 
text and description' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", 
' with parsable text and description' ],
+
// Bugs
[ 'null', "\t\r\n ", 'T127345: whitespace-only tag content, 
' ],
+   [ $xssJsonSanitized, "$xssJson", 'T134719: XSS via __proto__' ],
];
}
 

-- 
To view, visit https://gerrit.wikimedia.org/r/294650
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: wmf/1.28.0-wmf.6
Gerrit-Owner: Mattflaschen 
Gerrit-Reviewer: MaxSem 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] SECURITY: Fix XSS via __proto__ - change (mediawiki...Kartographer)

2016-06-14 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

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, 42 insertions(+), 9 deletions(-)

Approvals:
  MaxSem: Looks good to me, approved
  jenkins-bot: Verified



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 @@
 class SimpleStyleParser {
private static $parsedProps = [ 'title', 'description' ];
 
-   private static $recursedProps = [ 'geometry', 'geometries', 'features' 
];
-
/** @var Parser */
private $parser;
 
@@ -145,15 +143,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 4d5ad2e..47610c6 100644
--- a/tests/phpunit/KartographerTest.php
+++ b/tests/phpunit/KartographerTest.php
@@ -64,7 +64,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":"scriptalert(document.cookie);\/script",
@@ -79,8 +109,10 @@
[ 
"{\"_4622d19afa2e6480c327846395ed932ba6fa56d4\":[$validJson]}", "[$validJson]", ' with GeoJSON array' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", ' with parsable 
text and description' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", 
' with parsable text and description' ],
+
// Bugs
[ 'null', "\t\r\n ", 'T127345: whitespace-only tag content, 
' ],
+   [ $xssJsonSanitized, "$xssJson", 'T134719: XSS via __proto__' ],
];
}
 

-- 
To view, visit https://gerrit.wikimedia.org/r/294412
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Kartographer
Gerrit-Branch: REL1_27
Gerrit-Owner: MaxSem 
Gerrit-Reviewer: MaxSem 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] SECURITY: Fix XSS via __proto__ - change (mediawiki...Kartographer)

2016-06-14 Thread MaxSem (Code Review) 
MaxSem has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/294412

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, 42 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Kartographer 
refs/changes/12/294412/1

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 @@
 class SimpleStyleParser {
private static $parsedProps = [ 'title', 'description' ];
 
-   private static $recursedProps = [ 'geometry', 'geometries', 'features' 
];
-
/** @var Parser */
private $parser;
 
@@ -145,15 +143,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 4d5ad2e..47610c6 100644
--- a/tests/phpunit/KartographerTest.php
+++ b/tests/phpunit/KartographerTest.php
@@ -64,7 +64,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":"scriptalert(document.cookie);\/script",
@@ -79,8 +109,10 @@
[ 
"{\"_4622d19afa2e6480c327846395ed932ba6fa56d4\":[$validJson]}", "[$validJson]", ' with GeoJSON array' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", ' with parsable 
text and description' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", 
' with parsable text and description' ],
+
// Bugs
[ 'null', "\t\r\n ", 'T127345: whitespace-only tag content, 
' ],
+   [ $xssJsonSanitized, "$xssJson", 'T134719: XSS via __proto__' ],
];
}
 

-- 
To view, visit https://gerrit.wikimedia.org/r/294412
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: REL1_27
Gerrit-Owner: MaxSem 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] SECURITY: Fix XSS via __proto__ - change (mediawiki...Kartographer)

2016-06-14 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

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(-)

Approvals:
  MaxSem: Looks good to me, approved
  jenkins-bot: Verified



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":"scriptalert(document.cookie);\/script",
@@ -98,8 +127,10 @@
   }', 'Invalid JSON 6' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", ' with parsable 
text and description' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", 
' with parsable text and description' ],
+
// Bugs
[ 'null', "\t\r\n ", 'T127345: whitespace-only tag content, 
' ],
+   [ $xssJsonSanitized, "$xssJson", 'T134719: XSS via __proto__' ],
];
}
 

-- 
To view, visit https://gerrit.wikimedia.org/r/294373
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4b537f018c8d37491f46349ac9c764279123deb1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Kartographer
Gerrit-Branch: master
Gerrit-Owner: MaxSem 
Gerrit-Reviewer: MaxSem 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] SECURITY: Fix XSS via __proto__ - change (mediawiki...Kartographer)

2016-06-14 Thread MaxSem (Code Review) 
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":"scriptalert(document.cookie);\/script",
@@ -98,8 +127,10 @@
   }', 'Invalid JSON 6' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", ' with parsable 
text and description' ],
[ $wikitextJsonParsed, "[{$this->wikitextJson}]", 
' with parsable text and description' ],
+
// Bugs
[ 'null', "\t\r\n ", 'T127345: whitespace-only tag content, 
' ],
+   [ $xssJsonSanitized, "$xssJson", '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 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits