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":"<script>alert(document.cookie);<\/script>",
@@ -79,8 +109,10 @@
                        [ 
"{\"_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__' ],
                ];
        }
 

-- 
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 <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to