SVG upload with specific error (warning) message when blocking
authorUbuntu <ubuntu@ip-172-31-39-38.us-west-2.compute.internal>
Thu, 5 Mar 2015 08:02:34 +0000 (08:02 +0000)
committerBrian Wolff <bawolff+wn@gmail.com>
Mon, 25 May 2015 20:47:45 +0000 (20:47 +0000)
This patch is to generate specific error (warning) message when
blocking an svg file.

The checkSvgScriptCallback function has been updated, and it's
return type is changed from boolean to array.

A new variable is added to XmlTypeCheck class that contains the
type of error when svg file is uploaded, which is used to generate
concrete error messages later on.

I have added concrete error messages to i18n/en.json and their description
to qqq.json file. Please review the error messages and their description.

Bug: T85924
Change-Id: I3f687bf5b86ce66b703591b85fd03f073aacff4f

includes/libs/XmlTypeCheck.php
includes/upload/UploadBase.php
languages/i18n/en.json
languages/i18n/qqq.json

index 6d01986..34afb68 100644 (file)
@@ -38,6 +38,13 @@ class XmlTypeCheck {
         */
        public $filterMatch = false;
 
+       /**
+        * Will contain the type of filter hit if the optional element filter returned
+        * a match at some point.
+        * @var mixed
+        */
+       public $filterMatchType = false;
+
        /**
         * Name of the document's root element, including any namespace
         * as an expanded URL.
@@ -173,7 +180,7 @@ class XmlTypeCheck {
                // First, move through anything that isn't an element, and
                // handle any processing instructions with the callback
                do {
-                       if( !$this->readNext( $reader ) ) {
+                       if ( !$this->readNext( $reader ) ) {
                                // Hit the end of the document before any elements
                                $this->wellFormed = false;
                                return;
@@ -294,17 +301,20 @@ class XmlTypeCheck {
                list( $name, $attribs ) = array_pop( $this->elementDataContext );
                $data = array_pop( $this->elementData );
                $this->stackDepth--;
+               $callbackReturn = false;
 
-               if ( is_callable( $this->filterCallback )
-                       && call_user_func(
+               if ( is_callable( $this->filterCallback ) ) {
+                       $callbackReturn = call_user_func(
                                $this->filterCallback,
                                $name,
                                $attribs,
                                $data
-                       )
-               ) {
-                       // Filter hit
+                       );
+               }
+               if ( $callbackReturn ) {
+                       // Filter hit!
                        $this->filterMatch = true;
+                       $this->filterMatchType = $callbackReturn;
                }
        }
 
@@ -321,15 +331,18 @@ class XmlTypeCheck {
         * @param $data
         */
        private function processingInstructionHandler( $target, $data ) {
+               $callbackReturn = false;
                if ( $this->parserOptions['processing_instruction_handler'] ) {
-                       if ( call_user_func(
+                       $callbackReturn = call_user_func(
                                $this->parserOptions['processing_instruction_handler'],
                                $target,
                                $data
-                       ) ) {
-                               // Filter hit!
-                               $this->filterMatch = true;
-                       }
+                       );
+               }
+               if ( $callbackReturn ) {
+                       // Filter hit!
+                       $this->filterMatch = true;
+                       $this->filterMatchType = $callbackReturn;
                }
        }
 }
index 6da8250..df91588 100644 (file)
@@ -1266,7 +1266,7 @@ abstract class UploadBase {
                                return array( 'uploadscriptednamespace', $this->mSVGNSError );
                        }
 
-                       return array( 'uploadscripted' );
+                       return $check->filterMatchType;
                }
 
                return false;
@@ -1281,7 +1281,7 @@ abstract class UploadBase {
        public static function checkSvgPICallback( $target, $data ) {
                // Don't allow external stylesheets (bug 57550)
                if ( preg_match( '/xml-stylesheet/i', $target ) ) {
-                       return true;
+                       return array( 'upload-scripted-pi-callback' );
                }
 
                return false;
@@ -1353,7 +1353,7 @@ abstract class UploadBase {
                if ( $strippedElement == 'script' ) {
                        wfDebug( __METHOD__ . ": Found script element '$element' in uploaded file.\n" );
 
-                       return true;
+                       return array( 'uploaded-script-svg', $strippedElement );
                }
 
                # e.g., <svg xmlns="http://www.w3.org/2000/svg">
@@ -1361,21 +1361,21 @@ abstract class UploadBase {
                if ( $strippedElement == 'handler' ) {
                        wfDebug( __METHOD__ . ": Found scriptable element '$element' in uploaded file.\n" );
 
-                       return true;
+                       return array( 'uploaded-script-svg', $strippedElement );
                }
 
                # SVG reported in Feb '12 that used xml:stylesheet to generate javascript block
                if ( $strippedElement == 'stylesheet' ) {
                        wfDebug( __METHOD__ . ": Found scriptable element '$element' in uploaded file.\n" );
 
-                       return true;
+                       return array( 'uploaded-script-svg', $strippedElement );
                }
 
                # Block iframes, in case they pass the namespace check
                if ( $strippedElement == 'iframe' ) {
                        wfDebug( __METHOD__ . ": iframe in uploaded file.\n" );
 
-                       return true;
+                       return array( 'uploaded-script-svg', $strippedElement );
                }
 
                # Check <style> css
@@ -1383,7 +1383,7 @@ abstract class UploadBase {
                        && self::checkCssFragment( Sanitizer::normalizeCss( $data ) )
                ) {
                        wfDebug( __METHOD__ . ": hostile css in style element.\n" );
-                       return true;
+                       return array( 'uploaded-hostile-svg' );
                }
 
                foreach ( $attribs as $attrib => $value ) {
@@ -1394,7 +1394,7 @@ abstract class UploadBase {
                                wfDebug( __METHOD__
                                        . ": Found event-handler attribute '$attrib'='$value' in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-event-handler-on-svg', $attrib, $value );
                        }
 
                        # href with non-local target (don't allow http://, javascript:, etc)
@@ -1408,7 +1408,7 @@ abstract class UploadBase {
                                        wfDebug( __METHOD__ . ": Found href attribute <$strippedElement "
                                                . "'$attrib'='$value' in uploaded file.\n" );
 
-                                       return true;
+                                       return array( 'uploaded-href-attribute-svg', $strippedElement, $attrib, $value );
                                }
                        }
 
@@ -1420,7 +1420,7 @@ abstract class UploadBase {
                                if ( !preg_match( "!^data:\s*image/(gif|jpeg|jpg|png)$parameters,!i", $value ) ) {
                                        wfDebug( __METHOD__ . ": Found href to unwhitelisted data: uri "
                                                . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
-                                       return true;
+                                       return array( 'uploaded-href-unsafe-target-svg', $strippedElement, $attrib, $value );
                                }
                        }
 
@@ -1432,7 +1432,7 @@ abstract class UploadBase {
                                wfDebug( __METHOD__ . ": Found animate that might be changing href using from "
                                        . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-animate-svg', $strippedElement, $attrib, $value );
                        }
 
                        # use set/animate to add event-handler attribute to parent
@@ -1443,7 +1443,7 @@ abstract class UploadBase {
                                wfDebug( __METHOD__ . ": Found svg setting event-handler attribute with "
                                        . "\"<$strippedElement $stripped='$value'...\" in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-setting-event-handler-svg', $strippedElement, $stripped, $value );
                        }
 
                        # use set to add href attribute to parent element
@@ -1453,7 +1453,7 @@ abstract class UploadBase {
                        ) {
                                wfDebug( __METHOD__ . ": Found svg setting href attribute '$value' in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-setting-href-svg' );
                        }
 
                        # use set to add a remote / data / script target to an element
@@ -1463,7 +1463,7 @@ abstract class UploadBase {
                        ) {
                                wfDebug( __METHOD__ . ": Found svg setting attribute to '$value' in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-wrong-setting-svg', $value );
                        }
 
                        # use handler attribute with remote / data / script
@@ -1471,7 +1471,7 @@ abstract class UploadBase {
                                wfDebug( __METHOD__ . ": Found svg setting handler with remote/data/script "
                                        . "'$attrib'='$value' in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-setting-handler-svg', $attrib, $value );
                        }
 
                        # use CSS styles to bring in remote code
@@ -1480,7 +1480,7 @@ abstract class UploadBase {
                        ) {
                                wfDebug( __METHOD__ . ": Found svg setting a style with "
                                        . "remote url '$attrib'='$value' in uploaded file.\n" );
-                               return true;
+                               return array( 'uploaded-remote-url-svg', $attrib, $value );
                        }
 
                        # Several attributes can include css, css character escaping isn't allowed
@@ -1491,7 +1491,7 @@ abstract class UploadBase {
                        ) {
                                wfDebug( __METHOD__ . ": Found svg setting a style with "
                                        . "remote url '$attrib'='$value' in uploaded file.\n" );
-                               return true;
+                               return array( 'uploaded-remote-url-svg', $attrib, $value );
                        }
 
                        # image filters can pull in url, which could be svg that executes scripts
@@ -1502,7 +1502,7 @@ abstract class UploadBase {
                                wfDebug( __METHOD__ . ": Found image filter with url: "
                                        . "\"<$strippedElement $stripped='$value'...\" in uploaded file.\n" );
 
-                               return true;
+                               return array( 'uploaded-image-filter-svg', $strippedElement, $stripped, $value );
                        }
                }
 
index 9ad4d55..26c4b1b 100644 (file)
        "uploaddisabledtext": "File uploads are disabled.",
        "php-uploaddisabledtext": "File uploads are disabled in PHP.\nPlease check the file_uploads setting.",
        "uploadscripted": "This file contains HTML or script code that may be erroneously interpreted by a web browser.",
+       "upload-scripted-pi-callback": "Cannot upload a file that contains xml-stylesheet processing instruction.",
+       "uploaded-script-svg": "Found scriptable element \"$1\" in the uploaded SVG file.",
+       "uploaded-hostile-svg": "Found unsafe CSS in the style element of uploaded SVG file.",
+       "uploaded-event-handler-on-svg": "Setting event-handler attributes <code>$1=\"$2\"</code> is not allowed in SVG files.",
+       "uploaded-href-attribute-svg": "Href attributes <code>&lt;$1 $2=\"$3\"&gt;</code> with non-local target (e.g. http://, javascript:, etc) are not allowed in SVG files.",
+       "uploaded-href-unsafe-target-svg": "Found href to unsafe target <code>&lt;$1 $2=\"$3\"&gt;</code> in the uploaded SVG file.",
+       "uploaded-animate-svg": "Found \"animate\" tag that might be changing href, using the \"from\" attribute <code>&lt;$1 $2=\"$3\"&gt;</code> in the uploaded SVG file.",
+       "uploaded-setting-event-handler-svg": "Setting event-handler attributes is blocked, found <code>&lt;$1 $2=\"$3\"&gt;</code> in the uploaded SVG file.",
+       "uploaded-setting-href-svg": "Using the \"set\" tag to add \"href\" attribute to parent element is blocked.",
+       "uploaded-wrong-setting-svg": "Using the \"set\" tag to add a remote/data/script target to any attribute is blocked. Found <code>&lt;set to=\"$1\"&gt;</code> in the uploaded SVG file.",
+       "uploaded-setting-handler-svg": "SVG that sets the \"handler\" attribute with remote/data/script is blocked. Found <code>$1=\"$2\"</code> in the uploaded SVG file.",
+       "uploaded-remote-url-svg": "SVG that sets any style attribute with remote URL is blocked. Found <code>$1=\"$2\"</code> in the uploaded SVG file.",
+       "uploaded-image-filter-svg": "Found image filter with URL: <code>&lt;$1 $2=\"$3\"&gt;</code> in the uploaded SVG file.",
        "uploadscriptednamespace": "This SVG file contains an illegal namespace \"$1\".",
        "uploadinvalidxml": "The XML in the uploaded file could not be parsed.",
        "uploadvirus": "The file contains a virus!\nDetails: $1",
index 7b5cbc9..282ced5 100644 (file)
        "uploaddisabledtext": "Parameters:\n* $1 - (Optional) the name of the target file. See r22243 and [[bugzilla:8818|bug 8818]].",
        "php-uploaddisabledtext": "This means that file uploading is disabled in PHP, not upload of PHP-files.",
        "uploadscripted": "Used as error message when uploading a file.\n\nSee also:\n* {{msg-mw|zip-wrong-format}}\n* {{msg-mw|uploadjava}}\n* {{msg-mw|uploadvirus}}",
+       "upload-scripted-pi-callback": "Used as error message when uploading an SVG file that contains xml-stylesheet processing instruction.",
+       "uploaded-script-svg": "Used as error message when uploading an SVG file that contains scriptable tags (script, handler, stylesheet, iframe).\n\nParameters:\n* $1 - The scriptable tag that blocked the SVG file from uploading.",
+       "uploaded-hostile-svg": "Used as error message when uploading an SVG file that contains unsafe CSS.",
+       "uploaded-event-handler-on-svg": "Used as error message when uploading an SVG file that contains event-handler attributes.\n\nParameters:\n* $1 - The event-handler attribute that is being modified in the SVG file.\n* $2 - The value that is given to the event-handler attribute.",
+       "uploaded-href-attribute-svg": "Used as error message when uploading an SVG file that contains href attribute with non-local target (like http://, javascript:, etc).\n\nParameters:\n* $1 - The name of the tag containing href attribute.\n* $2 - The attribute \"href\".\n* $3 - The value of the href attribute.",
+       "uploaded-href-unsafe-target-svg": "Used as error message when uploading an SVG file that contains href to some unsafe target.\n\nParameters:\n* $1 - The name of the tag containing href attribute.\n* $2 - The attribute \"href\".\n* $3 - The value of the href attribute.",
+       "uploaded-animate-svg": "Used as error message when uploading an SVG file that contains the element <animate> that might be changing href.\n\nParameters:\n* $1 - The name of the HTML tag.\n* $2 - The name of the attribute.\n* $3 - The value getting assigned to the attribute.",
+       "uploaded-setting-event-handler-svg": "Used as error message when uploading an SVG file that sets the event-handler attribute, using <set> or <animate> tags.\n\nParameters:\n* $1 - The name of the HTML tag.\n* $2 - The name of the attribute.\n* $3 - The value getting assigned to the attribute.",
+       "uploaded-setting-href-svg": "Used as error message when uploading an SVG file that sets the href attribute, using the <set> tag.",
+       "uploaded-wrong-setting-svg": "Used as error message when uploading an SVG file that uses <set> tag to add a remote/data/script target, to an element.\n\nParameters:\n* $1 - The value of remote/data/script target.",
+       "uploaded-setting-handler-svg": "Used as error message when uploading an SVG file that sets the handler attribute with remote/data/script target.\n\nParameters:\n* $1 - The name of the attribute.\n* $2 - The value of the attribute.",
+       "uploaded-remote-url-svg": "Used as error message when uploading an SVG file that contains SVG setting some style attribute with remote URL.\n\nParameters:\n* $1 - The name of the attribute.\n* $2 - The value of the attribute.",
+       "uploaded-image-filter-svg": "Used as error message when uploading an SVG file that contains image filters, as they can pull in URL, which could be an SVG that executes scripts.\n\nParameters:\n* $1 - The name of the HTML tag.\n* $2 - The name of the attribute.\n* $3 - The value getting assigned to the attribute.",
        "uploadscriptednamespace": "Used as error message when uploading a file. This error is specific to SVG files, when they include a namespace that has not been whitelisted.\n\nParameters:\n* $1 - the invalid namespace name\nSee also:\n* {{msg-mw|zip-wrong-format}}\n* {{msg-mw|uploadjava}}\n* {{msg-mw|uploadvirus}}",
        "uploadinvalidxml": "Error message displayed when the uploaded file contains XML that cannot be properly parsed and checked.",
        "uploadvirus": "Error message displayed when uploaded file contains a virus.\n\nParameters:\n* $1 - {{msg-mw|Virus-unknownscanner}}, {{msg-mw|Virus-scanfailed}}, or something\nSee also:\n* {{msg-mw|Uploadscripted}}\n* {{msg-mw|Zip-wrong-format}}\n* {{msg-mw|Uploadjava}}",