SECURITY: Disallow stylesheets in svg

Bug: 57550
Change-Id: I73d148519c077e628d82a89280faa088bac9bdf5

diff --git a/includes/libs/XmlTypeCheck.php b/includes/libs/XmlTypeCheck.php
index 92ca7d8..eb98a4a 100644 (file)
--- a/includes/libs/XmlTypeCheck.php
+++ b/includes/libs/XmlTypeCheck.php
@@ -39,6 +39,13 @@ class XmlTypeCheck {
         */
        public $rootElement = '';
 
+       /**
+        * Additional parsing options
+        */
+       private $parserOptions = array(
+               'processing_instruction_handler' => '',
+       );
+
        /**
         * @param string $input a filename or string containing the XML element
         * @param callable $filterCallback (optional)
@@ -48,9 +55,13 @@ class XmlTypeCheck {
         *        Filter should return 'true' to toggle on $this->filterMatch
         * @param boolean $isFile (optional) indicates if the first parameter is a
         *        filename (default, true) or if it is a string (false)
+        * @param array $options list of additional parsing options:
+        *        processing_instruction_handler: Callback for xml_set_processing_instruction_handler
         */
-       function __construct( $input, $filterCallback = null, $isFile = true ) {
+       function __construct( $input, $filterCallback = null, $isFile = true, $options = array() ) {
                $this->filterCallback = $filterCallback;
+               $this->parserOptions = array_merge( $this->parserOptions, $options );
+
                if ( $isFile ) {
                        $this->validateFromFile( $input );
                } else {
@@ -107,6 +118,12 @@ class XmlTypeCheck {
                // case folding violates XML standard, turn it off
                xml_parser_set_option( $parser, XML_OPTION_CASE_FOLDING, false );
                xml_set_element_handler( $parser, array( $this, 'rootElementOpen' ), false );
+               if ( $this->parserOptions['processing_instruction_handler'] ) {
+                       xml_set_processing_instruction_handler(
+                               $parser,
+                               array( $this, 'processingInstructionHandler' )
+                       );
+               }
                return $parser;
        }
 
@@ -181,4 +198,16 @@ class XmlTypeCheck {
                        $this->filterMatch = true;
                }
        }
+
+       /**
+        * @param $parser
+        * @param $target
+        * @param $data
+        */
+       private function processingInstructionHandler( $parser, $target, $data ) {
+               if ( call_user_func( $this->parserOptions['processing_instruction_handler'], $target, $data ) ) {
+                       // Filter hit!
+                       $this->filterMatch = true;
+               }
+       }
 }

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 4c8148a..8ee36a6 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -1167,10 +1167,29 @@ abstract class UploadBase {
         * @return bool
         */
        protected function detectScriptInSvg( $filename ) {
-               $check = new XmlTypeCheck( $filename, array( $this, 'checkSvgScriptCallback' ) );
+               $check = new XmlTypeCheck(
+                       $filename,
+                       array( $this, 'checkSvgScriptCallback' ),
+                       true,
+                       array( 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback' )
+               );
                return $check->filterMatch;
        }
 
+       /**
+        * Callback to filter SVG Processing Instructions.
+        * @param $target string processing instruction name
+        * @param $data string processing instruction attribute and value
+        * @return bool (true if the filter identified something bad)
+        */
+       public static function checkSvgPICallback( $target, $data ) {
+               // Don't allow external stylesheets (bug 57550)
+               if ( preg_match( '/xml-stylesheet/i', $target) ) {
+                       return true;
+               }
+               return false;
+       }
+
        /**
         * @todo Replace this with a whitelist filter!
         * @param $element string