Merge "Revert "Localisation updates from https://translatewiki.net.""
authorL10n-bot <l10n-bot@translatewiki.net>
Thu, 6 Apr 2017 21:44:25 +0000 (21:44 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 6 Apr 2017 21:44:25 +0000 (21:44 +0000)
18 files changed:
RELEASE-NOTES-1.29
includes/EditPage.php
includes/api/ApiAuthManagerHelper.php
includes/api/ApiBase.php
includes/api/ApiCheckToken.php
includes/api/ApiLogin.php
includes/api/ApiMain.php
includes/api/ApiQueryWatchlist.php
includes/api/ApiQueryWatchlistRaw.php
includes/cache/localisation/LocalisationCache.php
includes/libs/mime/XmlTypeCheck.php
includes/parser/Parser.php
includes/parser/ParserOutput.php
includes/specials/SpecialWatchlist.php
includes/upload/UploadBase.php
languages/i18n/en.json
languages/i18n/qqq.json
tests/phpunit/includes/upload/UploadBaseTest.php

index a2dbcd5..b835eb5 100644 (file)
@@ -35,6 +35,8 @@ production.
 * (T156983) $wgRateLimitsExcludedIPs now accepts CIDR ranges as well as single IPs.
 * $wgDummyLanguageCodes is deprecated. Additional language code mappings may be
   added to $wgExtraLanguageCodes instead.
+* (T161453) LocalisationCache will no longer use the temporary directory in it's
+  fallback chain when trying to work out where to write the cache.
 
 === New features in 1.29 ===
 * (T5233) A cookie can now be set when a user is autoblocked, to track that user
@@ -90,6 +92,17 @@ production.
   to interwiki links.
 * (T144845) SECURITY: XSS in SearchHighlighter::highlightText() when
   $wgAdvancedSearchHighlighting is true.
+* (T125177) SECURITY: API parameters may now be marked as "sensitive" to keep
+  their values out of the logs.
+* (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.
+* (T161453) SECURITY: LocalisationCache will no longer use the temporary directory
+  in it's fallback chain when trying to work out where to write the cache.
+* (T48143) SECURITY: Spam blacklist ineffective on encoded URLs inside file inclusion
+  syntax's link parameter.
 
 === Action API changes in 1.29 ===
 * Submitting sensitive authentication request parameters to action=login,
@@ -150,6 +163,8 @@ production.
   various methods now take a module path rather than a module name.
 * ApiMessageTrait::getApiCode() now strips 'apierror-' and 'apiwarn-' prefixes
   from the message key, and maps some message keys for backwards compatibility.
+* API parameters may now be marked as "sensitive" to keep their values out of
+  the logs.
 
 === Languages updated in 1.29 ===
 
index e4d217c..2153b8c 100644 (file)
@@ -1027,7 +1027,7 @@ class EditPage {
                        throw new ErrorPageError(
                                'editpage-invalidcontentmodel-title',
                                'editpage-invalidcontentmodel-text',
-                               [ $this->contentModel ]
+                               [ wfEscapeWikiText( $this->contentModel ) ]
                        );
                }
 
@@ -1035,7 +1035,10 @@ class EditPage {
                        throw new ErrorPageError(
                                'editpage-notsupportedcontentformat-title',
                                'editpage-notsupportedcontentformat-text',
-                               [ $this->contentFormat, ContentHandler::getLocalizedName( $this->contentModel ) ]
+                               [
+                                       wfEscapeWikiText( $this->contentFormat ),
+                                       wfEscapeWikiText( ContentHandler::getLocalizedName( $this->contentModel ) )
+                               ]
                        );
                }
 
index d037c36..8862cc7 100644 (file)
@@ -169,6 +169,7 @@ class ApiAuthManagerHelper {
                $this->module->getMain()->markParamsUsed( array_keys( $data ) );
 
                if ( $sensitive ) {
+                       $this->module->getMain()->markParamsSensitive( array_keys( $sensitive ) );
                        $this->module->requirePostedParameters( array_keys( $sensitive ), 'noprefix' );
                }
 
index fec4234..b698cef 100644 (file)
@@ -188,6 +188,13 @@ abstract class ApiBase extends ContextSource {
         */
        const PARAM_EXTRA_NAMESPACES = 18;
 
+       /*
+        * (boolean) Is the parameter sensitive? Note 'password'-type fields are
+        * always sensitive regardless of the value of this field.
+        * @since 1.29
+        */
+       const PARAM_SENSITIVE = 19;
+
        /**@}*/
 
        const ALL_DEFAULT_STRING = '*';
@@ -1025,6 +1032,10 @@ abstract class ApiBase extends ContextSource {
                        } else {
                                $type = 'NULL'; // allow everything
                        }
+
+                       if ( $type == 'password' || !empty( $paramSettings[self::PARAM_SENSITIVE] ) ) {
+                               $this->getMain()->markParamsSensitive( $encParamName );
+                       }
                }
 
                if ( $type == 'boolean' ) {
@@ -2030,6 +2041,7 @@ abstract class ApiBase extends ContextSource {
                        $params['token'] = [
                                ApiBase::PARAM_TYPE => 'string',
                                ApiBase::PARAM_REQUIRED => true,
+                               ApiBase::PARAM_SENSITIVE => true,
                                ApiBase::PARAM_HELP_MSG => [
                                        'api-help-param-token',
                                        $this->needsToken(),
index 3cc7a8a..480915e 100644 (file)
@@ -73,6 +73,7 @@ class ApiCheckToken extends ApiBase {
                        'token' => [
                                ApiBase::PARAM_TYPE => 'string',
                                ApiBase::PARAM_REQUIRED => true,
+                               ApiBase::PARAM_SENSITIVE => true,
                        ],
                        'maxtokenage' => [
                                ApiBase::PARAM_TYPE => 'integer',
index e3513da..41bec35 100644 (file)
@@ -250,6 +250,7 @@ class ApiLogin extends ApiBase {
                        'token' => [
                                ApiBase::PARAM_TYPE => 'string',
                                ApiBase::PARAM_REQUIRED => false, // for BC
+                               ApiBase::PARAM_SENSITIVE => true,
                                ApiBase::PARAM_HELP_MSG => [ 'api-help-param-token', 'login' ],
                        ],
                ];
index a1fac0c..b5eff70 100644 (file)
@@ -161,6 +161,7 @@ class ApiMain extends ApiBase {
        private $mCacheMode = 'private';
        private $mCacheControl = [];
        private $mParamsUsed = [];
+       private $mParamsSensitive = [];
 
        /** @var bool|null Cached return value from self::lacksSameOriginSecurity() */
        private $lacksSameOriginSecurity = null;
@@ -1602,13 +1603,17 @@ class ApiMain extends ApiBase {
                        " {$logCtx['ip']} " .
                        "T={$logCtx['timeSpentBackend']}ms";
 
+               $sensitive = array_flip( $this->getSensitiveParams() );
                foreach ( $this->getParamsUsed() as $name ) {
                        $value = $request->getVal( $name );
                        if ( $value === null ) {
                                continue;
                        }
 
-                       if ( strlen( $value ) > 256 ) {
+                       if ( isset( $sensitive[$name] ) ) {
+                               $value = '[redacted]';
+                               $encValue = '[redacted]';
+                       } elseif ( strlen( $value ) > 256 ) {
                                $value = substr( $value, 0, 256 );
                                $encValue = $this->encodeRequestLogValue( $value ) . '[...]';
                        } else {
@@ -1658,6 +1663,24 @@ class ApiMain extends ApiBase {
                $this->mParamsUsed += array_fill_keys( (array)$params, true );
        }
 
+       /**
+        * Get the request parameters that should be considered sensitive
+        * @since 1.29
+        * @return array
+        */
+       protected function getSensitiveParams() {
+               return array_keys( $this->mParamsSensitive );
+       }
+
+       /**
+        * Mark parameters as sensitive
+        * @since 1.29
+        * @param string|string[] $params
+        */
+       public function markParamsSensitive( $params ) {
+               $this->mParamsSensitive += array_fill_keys( (array)$params, true );
+       }
+
        /**
         * Get a request value, and register the fact that it was used, for logging.
         * @param string $name
index fee0b78..f8f6e7d 100644 (file)
@@ -475,7 +475,8 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase {
                                ApiBase::PARAM_TYPE => 'user'
                        ],
                        'token' => [
-                               ApiBase::PARAM_TYPE => 'string'
+                               ApiBase::PARAM_TYPE => 'string',
+                               ApiBase::PARAM_SENSITIVE => true,
                        ],
                        'continue' => [
                                ApiBase::PARAM_HELP_MSG => 'api-help-param-continue',
index 116f219..b0b1cde 100644 (file)
@@ -170,7 +170,8 @@ class ApiQueryWatchlistRaw extends ApiQueryGeneratorBase {
                                ApiBase::PARAM_TYPE => 'user'
                        ],
                        'token' => [
-                               ApiBase::PARAM_TYPE => 'string'
+                               ApiBase::PARAM_TYPE => 'string',
+                               ApiBase::PARAM_SENSITIVE => true,
                        ],
                        'dir' => [
                                ApiBase::PARAM_DFLT => 'ascending',
index cbff113..d499340 100644 (file)
@@ -212,19 +212,17 @@ class LocalisationCache {
                                case 'detect':
                                        if ( !empty( $conf['storeDirectory'] ) ) {
                                                $storeClass = 'LCStoreCDB';
+                                       } elseif ( $wgCacheDirectory ) {
+                                               $storeConf['directory'] = $wgCacheDirectory;
+                                               $storeClass = 'LCStoreCDB';
                                        } else {
-                                               $cacheDir = $wgCacheDirectory ?: wfTempDir();
-                                               if ( $cacheDir ) {
-                                                       $storeConf['directory'] = $cacheDir;
-                                                       $storeClass = 'LCStoreCDB';
-                                               } else {
-                                                       $storeClass = 'LCStoreDB';
-                                               }
+                                               $storeClass = 'LCStoreDB';
                                        }
                                        break;
                                default:
                                        throw new MWException(
-                                               'Please set $wgLocalisationCacheConf[\'store\'] to something sensible.' );
+                                               'Please set $wgLocalisationCacheConf[\'store\'] to something sensible.'
+                                       );
                        }
                }
 
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 be4557d..953f021 100644 (file)
@@ -1610,9 +1610,7 @@ class Parser {
                                true, 'free',
                                $this->getExternalLinkAttribs( $url ), $this->mTitle );
                        # Register it in the output object...
-                       # Replace unnecessary URL escape codes with their equivalent characters
-                       $pasteurized = self::normalizeLinkUrl( $url );
-                       $this->mOutput->addExternalLink( $pasteurized );
+                       $this->mOutput->addExternalLink( $url );
                }
                return $text . $trail;
        }
@@ -1908,10 +1906,7 @@ class Parser {
                                $this->getExternalLinkAttribs( $url ), $this->mTitle ) . $dtrail . $trail;
 
                        # Register link in the output object.
-                       # Replace unnecessary URL escape codes with the referenced character
-                       # This prevents spammers from hiding links from the filters
-                       $pasteurized = self::normalizeLinkUrl( $url );
-                       $this->mOutput->addExternalLink( $pasteurized );
+                       $this->mOutput->addExternalLink( $url );
                }
 
                return $s;
@@ -5086,9 +5081,11 @@ class Parser {
                                                        }
                                                        if ( preg_match( "/^($prots)$addr$chars*$/u", $linkValue ) ) {
                                                                $link = $linkValue;
+                                                               $this->mOutput->addExternalLink( $link );
                                                        } else {
                                                                $localLinkTitle = Title::newFromText( $linkValue );
                                                                if ( $localLinkTitle !== null ) {
+                                                                       $this->mOutput->addLink( $localLinkTitle );
                                                                        $link = $localLinkTitle->getLinkURL();
                                                                }
                                                        }
index b2f99b3..7de3b30 100644 (file)
@@ -535,6 +535,10 @@ class ParserOutput extends CacheTime {
                # We don't register links pointing to our own server, unless... :-)
                global $wgServer, $wgRegisterInternalExternals;
 
+               # Replace unnecessary URL escape codes with the referenced character
+               # This prevents spammers from hiding links from the filters
+               $url = parser::normalizeLinkUrl( $url );
+
                $registerExternalLink = true;
                if ( !$wgRegisterInternalExternals ) {
                        $registerExternalLink = !self::isLinkInternal( $wgServer, $url );
index 365736f..c1c9ab0 100644 (file)
@@ -81,6 +81,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                if ( ( $config->get( 'EnotifWatchlist' ) || $config->get( 'ShowUpdatedMarker' ) )
                        && $request->getVal( 'reset' )
                        && $request->wasPosted()
+                       && $user->matchEditToken( $request->getVal( 'token' ) )
                ) {
                        $user->clearAllNotifications();
                        $output->redirect( $this->getPageTitle()->getFullURL( $opts->getChangedValues() ) );
@@ -660,6 +661,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                                'id' => 'mw-watchlist-resetbutton' ] ) . "\n" .
                        Xml::submitButton( $this->msg( 'enotif_reset' )->text(),
                                [ 'name' => 'mw-watchlist-reset-submit' ] ) . "\n" .
+                       Html::hidden( 'token', $user->getEditToken() ) . "\n" .
                        Html::hidden( 'reset', 'all' ) . "\n";
                        foreach ( $nondefaults as $key => $value ) {
                                $form .= Html::hidden( $key, $value ) . "\n";
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 ];
        }