SECURITY: Whitelist DTD declaration in SVG
authorBrian Wolff <bawolff+wn@gmail.com>
Mon, 28 Nov 2016 23:34:24 +0000 (23:34 +0000)
committerChad Horohoe <chadh@wikimedia.org>
Thu, 6 Apr 2017 20:43:04 +0000 (13:43 -0700)
Only allow ENTITY declarations inside the doctype internal
subset. Do not allow parameter entities, recursive entity
references are entity values longer than 255 bytes, or
external entity references. Filter external doctype subset
to only allow the standard svg doctypes.

Recursive entities that are simple aliases are allowed
because people appear to use them on commons. Declaring
xmlns:xlink to have a #FIXED value to the xlink namespace
is allowed because GraphViz apparently does that so its
somewhat common.

This prevents someone bypassing filter by using default
attribute values in internal dtd subset. No browser loads
the external dtd subset that I could find, but whitelist
just to be safe anyways.

Issue reported by Cassiogomes11.

Bug: T151735
Change-Id: I7cb4690f759ad97e70e06e560978b6207d84c446

RELEASE-NOTES-1.29
includes/libs/mime/XmlTypeCheck.php
includes/upload/UploadBase.php
languages/i18n/en.json
languages/i18n/qqq.json
tests/phpunit/includes/upload/UploadBaseTest.php

index 2552b40..25f72a8 100644 (file)
@@ -95,6 +95,8 @@ production.
 * (T150044) SECURITY: "Mark all pages visited" on the watchlist now requires a CSRF
   token.
 * (T156184) SECURITY: Escape content model/format url parameter in message.
+* (T151735) SECURITY: SVG filter evasion using default attribute values in DTD
+  declaration.
 
 === Action API changes in 1.29 ===
 * Submitting sensitive authentication request parameters to action=login,
index 7f2bf5e..e48cf62 100644 (file)
@@ -73,19 +73,36 @@ class XmlTypeCheck {
         */
        private $parserOptions = [
                'processing_instruction_handler' => '',
+               'external_dtd_handler' => '',
+               'dtd_handler' => '',
+               'require_safe_dtd' => true
        ];
 
        /**
+        * Allow filtering an XML file.
+        *
+        * Filters should return either true or a string to indicate something
+        * is wrong with the file. $this->filterMatch will store if the
+        * file failed validation (true = failed validation).
+        * $this->filterMatchType will contain the validation error.
+        * $this->wellFormed will contain whether the xml file is well-formed.
+        *
+        * @note If multiple filters are hit, only one of them will have the
+        *  result stored in $this->filterMatchType.
+        *
         * @param string $input a filename or string containing the XML element
         * @param callable $filterCallback (optional)
         *        Function to call to do additional custom validity checks from the
         *        SAX element handler event. This gives you access to the element
         *        namespace, name, attributes, and text contents.
-        *        Filter should return 'true' to toggle on $this->filterMatch
+        *        Filter should return a truthy value describing the error.
         * @param bool $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
+        *        external_dtd_handler: Callback for the url of external dtd subset
+        *        dtd_handler: Callback given the full text of the <!DOCTYPE declaration.
+        *        require_safe_dtd: Only allow non-recursive entities in internal dtd (default true)
         */
        function __construct( $input, $filterCallback = null, $isFile = true, $options = [] ) {
                $this->filterCallback = $filterCallback;
@@ -186,6 +203,9 @@ class XmlTypeCheck {
                        if ( $reader->nodeType === XMLReader::PI ) {
                                $this->processingInstructionHandler( $reader->name, $reader->value );
                        }
+                       if ( $reader->nodeType === XMLReader::DOC_TYPE ) {
+                               $this->DTDHandler( $reader );
+                       }
                } while ( $reader->nodeType != XMLReader::ELEMENT );
 
                // Process the rest of the document
@@ -234,8 +254,13 @@ class XmlTypeCheck {
                                                $reader->value
                                        );
                                        break;
+                               case XMLReader::DOC_TYPE:
+                                       // We should never see a doctype after first
+                                       // element.
+                                       $this->wellFormed = false;
+                                       break;
                                default:
-                                       // One of DOC, DOC_TYPE, ENTITY, END_ENTITY,
+                                       // One of DOC, ENTITY, END_ENTITY,
                                        // NOTATION, or XML_DECLARATION
                                        // xml_parse didn't send these to the filter, so we won't.
                        }
@@ -339,4 +364,140 @@ class XmlTypeCheck {
                        $this->filterMatchType = $callbackReturn;
                }
        }
+       /**
+        * Handle coming across a <!DOCTYPE declaration.
+        *
+        * @param XMLReader $reader Reader currently pointing at DOCTYPE node.
+        */
+       private function DTDHandler( XMLReader $reader ) {
+               $externalCallback = $this->parserOptions['external_dtd_handler'];
+               $generalCallback = $this->parserOptions['dtd_handler'];
+               $checkIfSafe = $this->parserOptions['require_safe_dtd'];
+               if ( !$externalCallback && !$generalCallback && !$checkIfSafe ) {
+                       return;
+               }
+               $dtd = $reader->readOuterXML();
+               $callbackReturn = false;
+
+               if ( $generalCallback ) {
+                       $callbackReturn = call_user_func( $generalCallback, $dtd );
+               }
+               if ( $callbackReturn ) {
+                       // Filter hit!
+                       $this->filterMatch = true;
+                       $this->filterMatchType = $callbackReturn;
+                       $callbackReturn = false;
+               }
+
+               $parsedDTD = $this->parseDTD( $dtd );
+               if ( $externalCallback && isset( $parsedDTD['type'] ) ) {
+                       $callbackReturn = call_user_func(
+                               $externalCallback,
+                               $parsedDTD['type'],
+                               isset( $parsedDTD['publicid'] ) ? $parsedDTD['publicid'] : null,
+                               isset( $parsedDTD['systemid'] ) ? $parsedDTD['systemid'] : null
+                       );
+               }
+               if ( $callbackReturn ) {
+                       // Filter hit!
+                       $this->filterMatch = true;
+                       $this->filterMatchType = $callbackReturn;
+                       $callbackReturn = false;
+               }
+
+               if ( $checkIfSafe && isset( $parsedDTD['internal'] ) ) {
+                       if ( !$this->checkDTDIsSafe( $parsedDTD['internal'] ) ) {
+                               $this->wellFormed = false;
+                       }
+               }
+       }
+
+       /**
+        * Check if the internal subset of the DTD is safe.
+        *
+        * We whitelist an extremely restricted subset of DTD features.
+        *
+        * Safe is defined as:
+        *  * Only contains entity defintions (e.g. No <!ATLIST )
+        *  * Entity definitions are not "system" entities
+        *  * Entity definitions are not "parameter" (i.e. %) entities
+        *  * Entity definitions do not reference other entites except &amp;
+        *    and quotes. Entity aliases (where the entity contains only
+        *    another entity are allowed)
+        *  * Entity references aren't overly long (>255 bytes).
+        *  * <!ATTLIST svg xmlns:xlink CDATA #FIXED "http://www.w3.org/1999/xlink">
+        *    allowed if matched exactly for compatibility with graphviz
+        *  * Comments.
+        *
+        * @param string $internalSubset The internal subset of the DTD
+        * @return bool true if safe.
+        */
+       private function checkDTDIsSafe( $internalSubset ) {
+               $offset = 0;
+               $res = preg_match(
+                       '/^(?:\s*<!ENTITY\s+\S+\s+' .
+                               '(?:"(?:&[^"%&;]{1,64};|(?:[^"%&]|&amp;|&quot;){0,255})"' .
+                               '|\'(?:&[^"%&;]{1,64};|(?:[^\'%&]|&amp;|&apos;){0,255})\')\s*>' .
+                               '|\s*<!--(?:[^-]|-[^-])*-->' .
+                               '|\s*<!ATTLIST svg xmlns:xlink CDATA #FIXED ' .
+                               '"http:\/\/www.w3.org\/1999\/xlink">)*\s*$/',
+                       $internalSubset
+               );
+
+               return (bool)$res;
+       }
+
+       /**
+        * Parse DTD into parts.
+        *
+        * If there is an error parsing the dtd, sets wellFormed to false.
+        *
+        * @param $dtd string
+        * @return array Possibly containing keys publicid, systemid, type and internal.
+        */
+       private function parseDTD( $dtd ) {
+               $m = [];
+               $res = preg_match(
+                       '/^<!DOCTYPE\s*\S+\s*' .
+                       '(?:(?P<typepublic>PUBLIC)\s*' .
+                               '(?:"(?P<pubquote>[^"]*)"|\'(?P<pubapos>[^\']*)\')' . // public identifer
+                               '\s*"(?P<pubsysquote>[^"]*)"|\'(?P<pubsysapos>[^\']*)\'' . // system identifier
+                       '|(?P<typesystem>SYSTEM)\s*' .
+                               '(?:"(?P<sysquote>[^"]*)"|\'(?P<sysapos>[^\']*)\')' .
+                       ')?\s*' .
+                       '(?:\[\s*(?P<internal>.*)\])?\s*>$/s',
+                       $dtd,
+                       $m
+               );
+               if ( !$res ) {
+                       $this->wellFormed = false;
+                       return [];
+               }
+               $parsed = [];
+               foreach ( $m as $field => $value ) {
+                       if ( $value === '' || is_numeric( $field ) ) {
+                               continue;
+                       }
+                       switch ( $field ) {
+                       case 'typepublic':
+                       case 'typesystem':
+                               $parsed['type'] = $value;
+                               break;
+                       case 'pubquote':
+                       case 'pubapos':
+                               $parsed['publicid'] = $value;
+                               break;
+                       case 'pubsysquote':
+                       case 'pubsysapos':
+                       case 'sysquote':
+                       case 'sysapos':
+                               $parsed['systemid'] = $value;
+                               break;
+                       case 'internal':
+                               $parsed['internal'] = $value;
+                               break;
+                       }
+               }
+               return $parsed;
+       }
 }
index 733c4ff..2c0afdf 100644 (file)
@@ -1359,7 +1359,10 @@ abstract class UploadBase {
                        $filename,
                        [ $this, 'checkSvgScriptCallback' ],
                        true,
-                       [ 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback' ]
+                       [
+                               'processing_instruction_handler' => 'UploadBase::checkSvgPICallback',
+                               'external_dtd_handler' => 'UploadBase::checkSvgExternalDTD',
+                       ]
                );
                if ( $check->wellFormed !== true ) {
                        // Invalid xml (T60553)
@@ -1391,6 +1394,34 @@ abstract class UploadBase {
                return false;
        }
 
+       /**
+        * Verify that DTD urls referenced are only the standard dtds
+        *
+        * Browsers seem to ignore external dtds. However just to be on the
+        * safe side, only allow dtds from the svg standard.
+        *
+        * @param string $type PUBLIC or SYSTEM
+        * @param string $publicId The well-known public identifier for the dtd
+        * @param string $systemId The url for the external dtd
+        */
+       public static function checkSvgExternalDTD( $type, $publicId, $systemId ) {
+               // This doesn't include the XHTML+MathML+SVG doctype since we don't
+               // allow XHTML anyways.
+               $allowedDTDs = [
+                       'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd',
+                       'http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd',
+                       'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-basic.dtd',
+                       'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11-tiny.dtd'
+               ];
+               if ( $type !== 'PUBLIC'
+                       || !in_array( $systemId, $allowedDTDs )
+                       || strpos( $publicId, "-//W3C//" ) !== 0
+               ) {
+                       return [ 'upload-scripted-dtd' ];
+               }
+               return false;
+       }
+
        /**
         * @todo Replace this with a whitelist filter!
         * @param string $element
index 9c4753c..a44ff92 100644 (file)
        "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.",
+       "upload-scripted-dtd": "Cannot upload SVG files that contain a non-standard DTD declaration.",
        "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.",
index ba99a55..5adfecd 100644 (file)
        "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.",
+       "upload-scripted-dtd": "Used as an error message when uploading an svg file that contains a DTD declaration where the system identifier of the DTD is for something other than the standard SVG DTDS, or it is a SYSTEM DTD, or the public identifier does not start with -//W3C//. Note that errors related to the internal dtd subset do not use this error message.",
        "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.",
index a42c86c..dd68cdc 100644 (file)
@@ -130,8 +130,8 @@ class UploadBaseTest extends MediaWikiTestCase {
         */
        public function testCheckSvgScriptCallback( $svg, $wellFormed, $filterMatch, $message ) {
                list( $formed, $match ) = $this->upload->checkSvgString( $svg );
-               $this->assertSame( $wellFormed, $formed, $message );
-               $this->assertSame( $filterMatch, $match, $message );
+               $this->assertSame( $wellFormed, $formed, $message . " (well-formed)" );
+               $this->assertSame( $filterMatch, $match, $message . " (filter match)" );
        }
 
        public static function provideCheckSvgScriptCallback() {
@@ -254,10 +254,16 @@ class UploadBaseTest extends MediaWikiTestCase {
                        ],
                        [
                                '<?xml version="1.0"?> <?xml-stylesheet type="text/xml" href="#stylesheet"?> <!DOCTYPE doc [ <!ATTLIST xsl:stylesheet id ID #REQUIRED>]> <svg xmlns="http://www.w3.org/2000/svg"> <xsl:stylesheet id="stylesheet" version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> <xsl:template match="/"> <iframe xmlns="http://www.w3.org/1999/xhtml" src="javascript:alert(1)"></iframe> </xsl:template> </xsl:stylesheet> <circle fill="red" r="40"></circle> </svg>',
-                               true,
+                               false,
                                true,
                                'SVG with embedded stylesheet (http://html5sec.org/#125)'
                        ],
+                       [
+                               '<?xml version="1.0"?> <?xml-stylesheet type="text/xml" href="#stylesheet"?> <svg xmlns="http://www.w3.org/2000/svg"> <xsl:stylesheet id="stylesheet" version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"> <xsl:template match="/"> <iframe xmlns="http://www.w3.org/1999/xhtml" src="javascript:alert(1)"></iframe> </xsl:template> </xsl:stylesheet> <circle fill="red" r="40"></circle> </svg>',
+                               true,
+                               true,
+                               'SVG with embedded stylesheet no doctype'
+                       ],
                        [
                                '<svg xmlns="http://www.w3.org/2000/svg" id="x"> <listener event="load" handler="#y" xmlns="http://www.w3.org/2001/xml-events" observer="x"/> <handler id="y">alert(1)</handler> </svg>',
                                true,
@@ -364,7 +370,7 @@ class UploadBaseTest extends MediaWikiTestCase {
                        ],
                        [
                                '<?xml version="1.0" encoding="UTF-8" standalone="no"?> <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [ <!ENTITY lol "lol"> <!ENTITY lol2 "&#x3C;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3E;&#x61;&#x6C;&#x65;&#x72;&#x74;&#x28;&#x27;&#x58;&#x53;&#x53;&#x45;&#x44;&#x20;&#x3D;&#x3E;&#x20;&#x27;&#x2B;&#x64;&#x6F;&#x63;&#x75;&#x6D;&#x65;&#x6E;&#x74;&#x2E;&#x64;&#x6F;&#x6D;&#x61;&#x69;&#x6E;&#x29;&#x3B;&#x3C;&#x2F;&#x73;&#x63;&#x72;&#x69;&#x70;&#x74;&#x3E;"> ]> <svg xmlns="http://www.w3.org/2000/svg" width="68" height="68" viewBox="-34 -34 68 68" version="1.1"> <circle cx="0" cy="0" r="24" fill="#c8c8c8"/> <text x="0" y="0" fill="black">&lol2;</text> </svg>',
-                               true,
+                               false,
                                true,
                                'SVG with encoded script tag in internal entity (reported by Beyond Security)'
                        ],
@@ -374,6 +380,16 @@ class UploadBaseTest extends MediaWikiTestCase {
                                false,
                                'SVG with external entity'
                        ],
+                       [
+                               // The base64 = <script>alert(1)</script>. If for some reason
+                               // entities actually do get loaded, this should trigger
+                               // filterMatch to be true. So this test verifies that we
+                               // are not loading external entities.
+                               '<?xml version="1.0"?> <!DOCTYPE svg [ <!ENTITY foo SYSTEM "data:text/plain;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pgo="> ]> <svg xmlns="http://www.w3.org/2000/svg" version="1.1"> <desc>&foo;</desc> <rect width="300" height="100" style="fill:rgb(0,0,255);stroke-width:1;stroke:rgb(0,0,2)" /> </svg>',
+                               false,
+                               false, /* False verifies entities aren't getting loaded */
+                               'SVG with data: uri external entity'
+                       ],
                        [
                                "<svg xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\"> <g> <a xlink:href=\"javascript:alert('1&#10;https://google.com')\"> <rect width=\"300\" height=\"100\" style=\"fill:rgb(0,0,255);stroke-width:1;stroke:rgb(0,0,2)\" /> </a> </g> </svg>",
                                true,
@@ -393,6 +409,104 @@ class UploadBaseTest extends MediaWikiTestCase {
                                false,
                                'SVG with local urls, including filter: in style'
                        ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE x [<!ATTLIST image x:href CDATA "data:image/png,foo" onerror CDATA "alert(\'XSSED = \'+document.domain)" onload CDATA "alert(\'XSSED = \'+document.domain)"> ]> <svg xmlns:h="http://www.w3.org/1999/xhtml" xmlns:x="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg"> <image /> </svg>',
+                               false,
+                               false,
+                               'SVG with evil default attribute values'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE svg SYSTEM "data:application/xml-dtd;base64,PCFET0NUWVBFIHN2ZyBbPCFBVFRMSVNUIGltYWdlIHg6aHJlZiBDREFUQSAiZGF0YTppbWFnZS9wbmcsZm9vIiBvbmVycm9yIENEQVRBICJhbGVydCgnWFNTRUQgPSAnK2RvY3VtZW50LmRvbWFpbikiIG9ubG9hZCBDREFUQSAiYWxlcnQoJ1hTU0VEID0gJytkb2N1bWVudC5kb21haW4pIj4gXT4K"><svg xmlns:x="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg"> <image /> </svg>',
+                               true,
+                               true,
+                               'SVG with an evil external dtd'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg PUBLIC "-//FOO/bar" "http://example.com"><svg></svg>',
+                               true,
+                               true,
+                               'SVG with random public doctype'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg SYSTEM \'http://example.com/evil.dtd\' ><svg></svg>',
+                               true,
+                               true,
+                               'SVG with random SYSTEM doctype'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg [<!ENTITY % foo "bar" >] ><svg></svg>',
+                               false,
+                               false,
+                               'SVG with parameter entity'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg [<!ENTITY foo "bar%a;" ] ><svg></svg>',
+                               false,
+                               false,
+                               'SVG with entity referencing parameter entity'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg [<!ENTITY foo "bar0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"> ] ><svg></svg>',
+                               false,
+                               false,
+                               'SVG with long entity'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg [<!ENTITY  foo \'"Hi", said bob\'> ] ><svg><g>&foo;</g></svg>',
+                               true,
+                               false,
+                               'SVG with apostrophe quote entity'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg [<!ENTITY name "Bob"><!ENTITY  foo \'"Hi", said &name;.\'> ] ><svg><g>&foo;</g></svg>',
+                               false,
+                               false,
+                               'SVG with recursive entity',
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.0//EN" "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd" [ <!ATTLIST svg xmlns:xlink CDATA #FIXED "http://www.w3.org/1999/xlink"> ]> <svg width="417pt" height="366pt"
+ viewBox="0.00 0.00 417.00 366.00" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"></svg>',
+                               true, /* well-formed */
+                               false, /* filter-hit */
+                               'GraphViz-esque svg with #FIXED xlink ns (Should be allowed)'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.0//EN" "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd" [ <!ATTLIST svg xmlns:xlink CDATA #FIXED "http://www.w3.org/1999/xlink2"> ]> <svg width="417pt" height="366pt"
+ viewBox="0.00 0.00 417.00 366.00" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"></svg>',
+                               false,
+                               false,
+                               'GraphViz ATLIST exception should match exactly'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [ <!-- Comment-here --> <!ENTITY foo "#ff6666">]><svg xmlns="http://www.w3.org/2000/svg"></svg>',
+                               true,
+                               false,
+                               'DTD with comments (Should be allowed)'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [ <!-- invalid--comment  --> <!ENTITY foo "#ff6666">]><svg xmlns="http://www.w3.org/2000/svg"></svg>',
+                               false,
+                               false,
+                               'DTD with invalid comment'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [ <!-- invalid ---> <!ENTITY foo "#ff6666">]><svg xmlns="http://www.w3.org/2000/svg"></svg>',
+                               false,
+                               false,
+                               'DTD with invalid comment 2'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [ <!ENTITY bar "&foo;"> <!ENTITY foo "#ff6666">]><svg xmlns="http://www.w3.org/2000/svg"></svg>',
+                               true,
+                               false,
+                               'DTD with aliased entities (Should be allowed)'
+                       ],
+                       [
+                               '<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" [ <!ENTITY bar \'&foo;\'> <!ENTITY foo \'#ff6666\'>]><svg xmlns="http://www.w3.org/2000/svg"></svg>',
+                               true,
+                               false,
+                               'DTD with aliased entities apos (Should be allowed)'
+                       ]
                ];
                // @codingStandardsIgnoreEnd
        }
@@ -478,7 +592,10 @@ class UploadTestHandler extends UploadBase {
                        $svg,
                        [ $this, 'checkSvgScriptCallback' ],
                        false,
-                       [ 'processing_instruction_handler' => 'UploadBase::checkSvgPICallback' ]
+                       [
+                               'processing_instruction_handler' => 'UploadBase::checkSvgPICallback',
+                               'external_dtd_handler' => 'UploadBase::checkSvgExternalDTD'
+                       ]
                );
                return [ $check->wellFormed, $check->filterMatch ];
        }