Merge "Use "string|false" as @return instead of "string|bool" where appropiate"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 1 Apr 2015 19:05:30 +0000 (19:05 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 1 Apr 2015 19:05:30 +0000 (19:05 +0000)
29 files changed:
RELEASE-NOTES-1.25
docs/hooks.txt
includes/DefaultSettings.php
includes/EditPage.php
includes/Html.php
includes/OutputPage.php
includes/User.php
includes/Xml.php
includes/api/ApiQuery.php
includes/installer/LocalSettingsGenerator.php
includes/libs/XmlTypeCheck.php
includes/media/BitmapMetadataHandler.php
includes/media/JpegMetadataExtractor.php
includes/media/XMP.php
includes/page/WikiPage.php
includes/specials/SpecialLinkSearch.php
includes/specials/SpecialUserlogin.php
includes/upload/UploadBase.php
languages/i18n/en.json
languages/i18n/qqq.json
resources/Resources.php
tests/parser/parserTests.txt
tests/phpunit/data/xmp/doctype-included.result.php [new file with mode: 0644]
tests/phpunit/data/xmp/doctype-included.xmp [new file with mode: 0644]
tests/phpunit/data/xmp/doctype-not-included.xmp [new file with mode: 0644]
tests/phpunit/includes/UserTest.php
tests/phpunit/includes/libs/XmlTypeCheckTest.php
tests/phpunit/includes/media/XMPTest.php
tests/phpunit/includes/upload/UploadBaseTest.php

index 0bf7a80..a31461c 100644 (file)
@@ -408,6 +408,8 @@ changes to languages because of Bugzilla reports.
   addSecondaryDataUpdate throwing an exception. These functions will be removed in 1.26,
   since they interfere with caching of ParserOutput objects.
 * Introduced new hook 'SecondaryDataUpdates' that allows extensions to inject custom updates.
+* Introduced new hook 'OpportunisticLinksUpdate' that allows extensions to perform
+  updates when a page is re-rendered.
 * EditPage::attemptSave has been modified not to call handleStatus itself and
   instead just returns the Status object. Extension calling it should be aware of
   this.
index f2c47ca..877b7ed 100644 (file)
@@ -2012,6 +2012,17 @@ return false to omit the line from RecentChanges and Watchlist special pages.
 can alter or append to the array of URLs for search & suggestion formats.
 &$urls: array of associative arrays with Url element attributes
 
+'OpportunisticLinksUpdate': Called by WikiPage::triggerOpportunisticLinksUpdate
+when a page view triggers a re-rendering of the page. This may happen
+particularly if the parser cache is split by user language, and no cached
+rendering of the page exists in the user's language. The hook is called
+before checking whether page_links_updated indicates that the links are up
+to date. Returning false will cause triggerOpportunisticLinksUpdate() to abort
+without triggering any updates.
+$page: the Page that was rendered.
+$title: the Title of the rendered page.
+$parserOutput: ParserOutput resulting from rendering the page.
+
 'OtherBlockLogLink': Get links to the block log from extensions which blocks
 users and/or IP addresses too.
 $otherBlockLink: An array with links to other block logs
index 5ab557e..84dc3aa 100644 (file)
@@ -4226,6 +4226,18 @@ $wgPasswordSalt = true;
  */
 $wgMinimalPasswordLength = 1;
 
+/**
+ * Specifies the maximal length of a user password (T64685).
+ *
+ * It is not recommended to make this greater than the default, as it can
+ * allow DoS attacks by users setting really long passwords. In addition,
+ * this should not be lowered too much, as it enforces weak passwords.
+ *
+ * @warning Unlike other password settings, user with passwords greater than
+ *      the maximum will not be able to log in.
+ */
+$wgMaximalPasswordLength = 4096;
+
 /**
  * Specifies if users should be sent to a password-reset form on login, if their
  * password doesn't meet the requirements of User::isValidPassword().
index a5994e7..e113426 100644 (file)
@@ -2670,19 +2670,21 @@ class EditPage {
                                                array( 'userinvalidcssjstitle', $this->mTitle->getSkinFromCssJsSubpage() )
                                        );
                                }
-                               if ( $this->formtype !== 'preview' ) {
-                                       if ( $this->isCssSubpage && $wgAllowUserCss ) {
-                                               $wgOut->wrapWikiMsg(
-                                                       "<div id='mw-usercssyoucanpreview'>\n$1\n</div>",
-                                                       array( 'usercssyoucanpreview' )
-                                               );
-                                       }
+                               if ( $this->getTitle()->isSubpageOf( $wgUser->getUserPage() ) ) {
+                                       if ( $this->formtype !== 'preview' ) {
+                                               if ( $this->isCssSubpage && $wgAllowUserCss ) {
+                                                       $wgOut->wrapWikiMsg(
+                                                               "<div id='mw-usercssyoucanpreview'>\n$1\n</div>",
+                                                               array( 'usercssyoucanpreview' )
+                                                       );
+                                               }
 
-                                       if ( $this->isJsSubpage && $wgAllowUserJs ) {
-                                               $wgOut->wrapWikiMsg(
-                                                       "<div id='mw-userjsyoucanpreview'>\n$1\n</div>",
-                                                       array( 'userjsyoucanpreview' )
-                                               );
+                                               if ( $this->isJsSubpage && $wgAllowUserJs ) {
+                                                       $wgOut->wrapWikiMsg(
+                                                               "<div id='mw-userjsyoucanpreview'>\n$1\n</div>",
+                                                               array( 'userjsyoucanpreview' )
+                                                       );
+                                               }
                                        }
                                }
                        }
index 4b69885..effc488 100644 (file)
@@ -600,17 +600,20 @@ class Html {
                        } else {
                                // Apparently we need to entity-encode \n, \r, \t, although the
                                // spec doesn't mention that.  Since we're doing strtr() anyway,
-                               // and we don't need <> escaped here, we may as well not call
-                               // htmlspecialchars().
+                               // we may as well not call htmlspecialchars().
                                // @todo FIXME: Verify that we actually need to
                                // escape \n\r\t here, and explain why, exactly.
                                #
                                // We could call Sanitizer::encodeAttribute() for this, but we
                                // don't because we're stubborn and like our marginal savings on
                                // byte size from not having to encode unnecessary quotes.
+                               // The only difference between this transform and the one by
+                               // Sanitizer::encodeAttribute() is '<' is only encoded here if
+                               // $wgWellFormedXml is set, and ' is not encoded.
                                $map = array(
                                        '&' => '&amp;',
                                        '"' => '&quot;',
+                                       '>' => '&gt;',
                                        "\n" => '&#10;',
                                        "\r" => '&#13;',
                                        "\t" => '&#9;'
index edeae0d..cac89f4 100644 (file)
@@ -3106,7 +3106,7 @@ class OutputPage extends ContextSource {
                // This also enforces $.isReady to be true at </body> which fixes the
                // mw.loader bug in Firefox with using document.write between </body>
                // and the DOMContentReady event (bug 47457).
-               $html = Html::inlineScript( 'window.jQuery && jQuery.ready();' );
+               $html = Html::inlineScript( 'if(window.jQuery)jQuery.ready();' );
 
                if ( !$this->getConfig()->get( 'ResourceLoaderExperimentalAsyncLoading' ) ) {
                        $html .= $this->getScriptsForBottomQueue( false );
@@ -3288,6 +3288,10 @@ class OutputPage extends ContextSource {
                if ( !$this->getTitle()->isJsSubpage() && !$this->getTitle()->isCssSubpage() ) {
                        return false;
                }
+               if ( !$this->getTitle()->isSubpageOf( $this->getUser()->getUserPage() ) ) {
+                       // Don't execute another user's CSS or JS on preview (T85855)
+                       return false;
+               }
 
                return !count( $this->getTitle()->getUserPermissionsErrors( 'edit', $this->getUser() ) );
        }
index 89ff299..2e88978 100644 (file)
@@ -826,15 +826,24 @@ class User implements IDBAccessObject {
        }
 
        /**
-        * Check if this is a valid password for this user. Status will be good if
-        * the password is valid, or have an array of error messages if not.
+        * Check if this is a valid password for this user
+        *
+        * Create a Status object based on the password's validity.
+        * The Status should be set to fatal if the user should not
+        * be allowed to log in, and should have any errors that
+        * would block changing the password.
+        *
+        * If the return value of this is not OK, the password
+        * should not be checked. If the return value is not Good,
+        * the password can be checked, but the user should not be
+        * able to set their password to this.
         *
         * @param string $password Desired password
         * @return Status
         * @since 1.23
         */
        public function checkPasswordValidity( $password ) {
-               global $wgMinimalPasswordLength, $wgContLang;
+               global $wgMinimalPasswordLength, $wgMaximalPasswordLength, $wgContLang;
 
                static $blockedLogins = array(
                        'Useruser' => 'Passpass', 'Useruser1' => 'Passpass1', # r75589
@@ -854,6 +863,10 @@ class User implements IDBAccessObject {
                        if ( strlen( $password ) < $wgMinimalPasswordLength ) {
                                $status->error( 'passwordtooshort', $wgMinimalPasswordLength );
                                return $status;
+                       } elseif ( strlen( $password ) > $wgMaximalPasswordLength ) {
+                               // T64685: Password too long, might cause DoS attack
+                               $status->fatal( 'passwordtoolong', $wgMaximalPasswordLength );
+                               return $status;
                        } elseif ( $wgContLang->lc( $password ) == $wgContLang->lc( $this->mName ) ) {
                                $status->error( 'password-name-match' );
                                return $status;
@@ -2382,17 +2395,9 @@ class User implements IDBAccessObject {
                                throw new PasswordError( wfMessage( 'password-change-forbidden' )->text() );
                        }
 
-                       if ( !$this->isValidPassword( $str ) ) {
-                               global $wgMinimalPasswordLength;
-                               $valid = $this->getPasswordValidity( $str );
-                               if ( is_array( $valid ) ) {
-                                       $message = array_shift( $valid );
-                                       $params = $valid;
-                               } else {
-                                       $message = $valid;
-                                       $params = array( $wgMinimalPasswordLength );
-                               }
-                               throw new PasswordError( wfMessage( $message, $params )->text() );
+                       $status = $this->checkPasswordValidity( $str );
+                       if ( !$status->isGood() ) {
+                               throw new PasswordError( $status->getMessage()->text() );
                        }
                }
 
@@ -3900,6 +3905,13 @@ class User implements IDBAccessObject {
 
                $this->loadPasswords();
 
+               // Some passwords will give a fatal Status, which means there is
+               // some sort of technical or security reason for this password to
+               // be completely invalid and should never be checked (e.g., T64685)
+               if ( !$this->checkPasswordValidity( $password )->isOK() ) {
+                       return false;
+               }
+
                // Certain authentication plugins do NOT want to save
                // domain passwords in a mysql database, so we should
                // check this (in case $wgAuth->strict() is false).
index 78b8715..f0bd70b 100644 (file)
@@ -703,13 +703,15 @@ class Xml {
        /**
         * Check if a string is well-formed XML.
         * Must include the surrounding tag.
+        * This function is a DoS vector if an attacker can define
+        * entities in $text.
         *
         * @param string $text String to test.
         * @return bool
         *
         * @todo Error position reporting return
         */
-       public static function isWellFormed( $text ) {
+       private static function isWellFormed( $text ) {
                $parser = xml_parser_create( "UTF-8" );
 
                # case folding violates XML standard, turn it off
index f4b64a3..ac89419 100644 (file)
@@ -249,16 +249,6 @@ class ApiQuery extends ApiBase {
        public function execute() {
                $this->mParams = $this->extractRequestParams();
 
-               if ( $this->mParams['continue'] === null && !$this->mParams['rawcontinue'] ) {
-                       $this->logFeatureUsage( 'action=query&!rawcontinue&!continue' );
-                       $this->setWarning(
-                               'Formatting of continuation data will be changing soon. ' .
-                               'To continue using the current formatting, use the \'rawcontinue\' parameter. ' .
-                               'To begin using the new format, pass an empty string for \'continue\' ' .
-                               'in the initial query.'
-                       );
-               }
-
                // Instantiate requested modules
                $allModules = array();
                $this->instantiateModules( $allModules, 'prop' );
@@ -304,6 +294,18 @@ class ApiQuery extends ApiBase {
                $this->getResult()->endContinuation(
                        $this->mParams['continue'] === null ? 'raw' : 'standard'
                );
+
+               if ( $this->mParams['continue'] === null && !$this->mParams['rawcontinue'] &&
+                       array_key_exists( 'query-continue', $this->getResult()->getData() )
+               ) {
+                       $this->logFeatureUsage( 'action=query&!rawcontinue&!continue' );
+                       $this->setWarning(
+                               'Formatting of continuation data will be changing soon. ' .
+                               'To continue using the current formatting, use the \'rawcontinue\' parameter. ' .
+                               'To begin using the new format, pass an empty string for \'continue\' ' .
+                               'in the initial query.'
+                       );
+               }
        }
 
        /**
index 8724e0d..3ba5e37 100644 (file)
@@ -143,8 +143,7 @@ class LocalSettingsGenerator {
 # The following skins were automatically enabled:\n";
 
                        foreach ( $this->skins as $skinName ) {
-                               $encSkinName = self::escapePhpString( $skinName );
-                               $localSettings .= "require_once \"\$IP/skins/$encSkinName/$encSkinName.php\";\n";
+                               $localSettings .= $this->generateRequireOnceLine( 'skins', $skinName );
                        }
 
                        $localSettings .= "\n";
@@ -157,8 +156,7 @@ class LocalSettingsGenerator {
 # The following extensions were automatically enabled:\n";
 
                        foreach ( $this->extensions as $extName ) {
-                               $encExtName = self::escapePhpString( $extName );
-                               $localSettings .= "require_once \"\$IP/extensions/$encExtName/$encExtName.php\";\n";
+                               $localSettings .= $this->generateRequireOnceLine( 'extensions', $extName );
                        }
 
                        $localSettings .= "\n";
@@ -171,6 +169,16 @@ class LocalSettingsGenerator {
                return $localSettings;
        }
 
+       /**
+        * @param string $dir Either "extensions" or "skins"
+        * @param string $name Name of extension/skin
+        * @return string
+        */
+       private function generateRequireOnceLine( $dir, $name ) {
+               $encName = self::escapePhpString( $name );
+               return "require_once \"\$IP/$dir/$encName/$encName.php\";\n";
+       }
+
        /**
         * Write the generated LocalSettings to a file
         *
index 0d6c3a6..6d01986 100644 (file)
@@ -2,6 +2,11 @@
 /**
  * XML syntax and type checker.
  *
+ * Since 1.24.2, it uses XMLReader instead of xml_parse, which gives us
+ * more control over the expansion of XML entities. When passed to the
+ * callback, entities will be fully expanded, but may report the XML is
+ * invalid if expanding the entities are likely to cause a DoS.
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -25,7 +30,7 @@ class XmlTypeCheck {
         * Will be set to true or false to indicate whether the file is
         * well-formed XML. Note that this doesn't check schema validity.
         */
-       public $wellFormed = false;
+       public $wellFormed = null;
 
        /**
         * Will be set to true if the optional element filter returned
@@ -78,12 +83,7 @@ class XmlTypeCheck {
        function __construct( $input, $filterCallback = null, $isFile = true, $options = array() ) {
                $this->filterCallback = $filterCallback;
                $this->parserOptions = array_merge( $this->parserOptions, $options );
-
-               if ( $isFile ) {
-                       $this->validateFromFile( $input );
-               } else {
-                       $this->validateFromString( $input );
-               }
+               $this->validateFromInput( $input, $isFile );
        }
 
        /**
@@ -125,140 +125,211 @@ class XmlTypeCheck {
                return $this->rootElement;
        }
 
+
        /**
-        * Get an XML parser with the root element handler.
-        * @see XmlTypeCheck::rootElementOpen()
-        * @return resource a resource handle for the XML parser
+        * @param string $fname the filename
         */
-       private function getParser() {
-               $parser = xml_parser_create_ns( 'UTF-8' );
-               // case folding violates XML standard, turn it off
-               xml_parser_set_option( $parser, XML_OPTION_CASE_FOLDING, false );
-               xml_set_element_handler( $parser, array( $this, 'rootElementOpen' ), false );
-               if ( $this->parserOptions['processing_instruction_handler'] ) {
-                       xml_set_processing_instruction_handler(
-                               $parser,
-                               array( $this, 'processingInstructionHandler' )
-                       );
+       private function validateFromInput( $xml, $isFile ) {
+               $reader = new XMLReader();
+               if ( $isFile ) {
+                       $s = $reader->open( $xml, null, LIBXML_NOERROR | LIBXML_NOWARNING );
+               } else {
+                       $s = $reader->XML( $xml, null, LIBXML_NOERROR | LIBXML_NOWARNING );
+               }
+               if ( $s !== true ) {
+                       // Couldn't open the XML
+                       $this->wellFormed = false;
+               } else {
+                       $oldDisable = libxml_disable_entity_loader( true );
+                       $reader->setParserProperty( XMLReader::SUBST_ENTITIES, true );
+                       try {
+                               $this->validate( $reader );
+                       } catch ( Exception $e ) {
+                               // Calling this malformed, because we didn't parse the whole
+                               // thing. Maybe just an external entity refernce.
+                               $this->wellFormed = false;
+                               $reader->close();
+                               libxml_disable_entity_loader( $oldDisable );
+                               throw $e;
+                       }
+                       $reader->close();
+                       libxml_disable_entity_loader( $oldDisable );
                }
-               return $parser;
        }
 
-       /**
-        * @param string $fname the filename
-        */
-       private function validateFromFile( $fname ) {
-               $parser = $this->getParser();
-
-               if ( file_exists( $fname ) ) {
-                       $file = fopen( $fname, "rb" );
-                       if ( $file ) {
-                               do {
-                                       $chunk = fread( $file, 32768 );
-                                       $ret = xml_parse( $parser, $chunk, feof( $file ) );
-                                       if ( $ret == 0 ) {
-                                               $this->wellFormed = false;
-                                               fclose( $file );
-                                               xml_parser_free( $parser );
-                                               return;
+       private function readNext( XMLReader $reader ) {
+               set_error_handler( array( $this, 'XmlErrorHandler' ) );
+               $ret = $reader->read();
+               restore_error_handler();
+               return $ret;
+       }
+
+       public function XmlErrorHandler( $errno, $errstr ) {
+               $this->wellFormed = false;
+       }
+
+       private function validate( $reader ) {
+
+               // First, move through anything that isn't an element, and
+               // handle any processing instructions with the callback
+               do {
+                       if( !$this->readNext( $reader ) ) {
+                               // Hit the end of the document before any elements
+                               $this->wellFormed = false;
+                               return;
+                       }
+                       if ( $reader->nodeType === XMLReader::PI ) {
+                               $this->processingInstructionHandler( $reader->name, $reader->value );
+                       }
+               } while ( $reader->nodeType != XMLReader::ELEMENT );
+
+               // Process the rest of the document
+               do {
+                       switch ( $reader->nodeType ) {
+                               case XMLReader::ELEMENT:
+                                       $name = $this->expandNS(
+                                               $reader->name,
+                                               $reader->namespaceURI
+                                       );
+                                       if ( $this->rootElement === '' ) {
+                                               $this->rootElement = $name;
                                        }
-                               } while ( !feof( $file ) );
+                                       $empty = $reader->isEmptyElement;
+                                       $attrs = $this->getAttributesArray( $reader );
+                                       $this->elementOpen( $name, $attrs );
+                                       if ( $empty ) {
+                                               $this->elementClose();
+                                       }
+                                       break;
+
+                               case XMLReader::END_ELEMENT:
+                                       $this->elementClose();
+                                       break;
+
+                               case XMLReader::WHITESPACE:
+                               case XMLReader::SIGNIFICANT_WHITESPACE:
+                               case XMLReader::CDATA:
+                               case XMLReader::TEXT:
+                                       $this->elementData( $reader->value );
+                                       break;
 
-                               fclose( $file );
+                               case XMLReader::ENTITY_REF:
+                                       // Unexpanded entity (maybe external?),
+                                       // don't send to the filter (xml_parse didn't)
+                                       break;
+
+                               case XMLReader::COMMENT:
+                                       // Don't send to the filter (xml_parse didn't)
+                                       break;
+
+                               case XMLReader::PI:
+                                       // Processing instructions can happen after the header too
+                                       $this->processingInstructionHandler(
+                                               $reader->name,
+                                               $reader->value
+                                       );
+                                       break;
+                               default:
+                                       // One of DOC, DOC_TYPE, ENTITY, END_ENTITY,
+                                       // NOTATION, or XML_DECLARATION
+                                       // xml_parse didn't send these to the filter, so we won't.
                        }
+
+               } while ( $this->readNext( $reader ) );
+
+               if ( $this->stackDepth !== 0 ) {
+                       $this->wellFormed = false;
+               } elseif ( $this->wellFormed === null ) {
+                       $this->wellFormed = true;
                }
-               $this->wellFormed = true;
 
-               xml_parser_free( $parser );
        }
 
        /**
-        *
-        * @param string $string the XML-input-string to be checked.
+        * Get all of the attributes for an XMLReader's current node
+        * @param $r XMLReader
+        * @return array of attributes
         */
-       private function validateFromString( $string ) {
-               $parser = $this->getParser();
-               $ret = xml_parse( $parser, $string, true );
-               xml_parser_free( $parser );
-               if ( $ret == 0 ) {
-                       $this->wellFormed = false;
-                       return;
+       private function getAttributesArray( XMLReader $r ) {
+               $attrs = array();
+               while ( $r->moveToNextAttribute() ) {
+                       if ( $r->namespaceURI === 'http://www.w3.org/2000/xmlns/' ) {
+                               // XMLReader treats xmlns attributes as normal
+                               // attributes, while xml_parse doesn't
+                               continue;
+                       }
+                       $name = $this->expandNS( $r->name, $r->namespaceURI );
+                       $attrs[$name] = $r->value;
                }
-               $this->wellFormed = true;
+               return $attrs;
        }
 
        /**
-        * @param $parser
-        * @param $name
-        * @param $attribs
+        * @param $name element or attribute name, maybe with a full or short prefix
+        * @param $namespaceURI the namespaceURI
+        * @return string the name prefixed with namespaceURI
         */
-       private function rootElementOpen( $parser, $name, $attribs ) {
-               $this->rootElement = $name;
-
-               if ( is_callable( $this->filterCallback ) ) {
-                       xml_set_element_handler(
-                               $parser,
-                               array( $this, 'elementOpen' ),
-                               array( $this, 'elementClose' )
-                       );
-                       xml_set_character_data_handler( $parser, array( $this, 'elementData' ) );
-                       $this->elementOpen( $parser, $name, $attribs );
-               } else {
-                       // We only need the first open element
-                       xml_set_element_handler( $parser, false, false );
+       private function expandNS( $name, $namespaceURI ) {
+               if ( $namespaceURI ) {
+                       $parts = explode( ':', $name );
+                       $localname = array_pop( $parts );
+                       return "$namespaceURI:$localname";
                }
+               return $name;
        }
 
        /**
-        * @param $parser
         * @param $name
         * @param $attribs
         */
-       private function elementOpen( $parser, $name, $attribs ) {
+       private function elementOpen( $name, $attribs ) {
                $this->elementDataContext[] = array( $name, $attribs );
                $this->elementData[] = '';
                $this->stackDepth++;
        }
 
        /**
-        * @param $parser
-        * @param $name
         */
-       private function elementClose( $parser, $name ) {
+       private function elementClose() {
                list( $name, $attribs ) = array_pop( $this->elementDataContext );
                $data = array_pop( $this->elementData );
                $this->stackDepth--;
 
-               if ( call_user_func(
-                       $this->filterCallback,
-                       $name,
-                       $attribs,
-                       $data
-               ) ) {
-                       // Filter hit!
+               if ( is_callable( $this->filterCallback )
+                       && call_user_func(
+                               $this->filterCallback,
+                               $name,
+                               $attribs,
+                               $data
+                       )
+               ) {
+                       // Filter hit
                        $this->filterMatch = true;
                }
        }
 
        /**
-        * @param $parser
         * @param $data
         */
-       private function elementData( $parser, $data ) {
-               // xml_set_character_data_handler breaks the data on & characters, so
-               // we collect any data here, and we'll run the callback in elementClose
+       private function elementData( $data ) {
+               // Collect any data here, and we'll run the callback in elementClose
                $this->elementData[ $this->stackDepth - 1 ] .= trim( $data );
        }
 
        /**
-        * @param $parser
         * @param $target
         * @param $data
         */
-       private function processingInstructionHandler( $parser, $target, $data ) {
-               if ( call_user_func( $this->parserOptions['processing_instruction_handler'], $target, $data ) ) {
-                       // Filter hit!
-                       $this->filterMatch = true;
+       private function processingInstructionHandler( $target, $data ) {
+               if ( $this->parserOptions['processing_instruction_handler'] ) {
+                       if ( call_user_func(
+                               $this->parserOptions['processing_instruction_handler'],
+                               $target,
+                               $data
+                       ) ) {
+                               // Filter hit!
+                               $this->filterMatch = true;
+                       }
                }
        }
 }
index bb7a1e8..c8d37bb 100644 (file)
@@ -154,7 +154,7 @@ class BitmapMetadataHandler {
         * @throws MWException On invalid file.
         */
        static function Jpeg( $filename ) {
-               $showXMP = function_exists( 'xml_parser_create_ns' );
+               $showXMP = XMPReader::isSupported();
                $meta = new self();
 
                $seg = JpegMetadataExtractor::segmentSplitter( $filename );
@@ -196,7 +196,7 @@ class BitmapMetadataHandler {
         * @return array Array for storage in img_metadata.
         */
        public static function PNG( $filename ) {
-               $showXMP = function_exists( 'xml_parser_create_ns' );
+               $showXMP = XMPReader::isSupported();
 
                $meta = new self();
                $array = PNGMetadataExtractor::getMetadata( $filename );
@@ -236,7 +236,7 @@ class BitmapMetadataHandler {
                        $meta->addMetadata( array( 'GIFFileComment' => $baseArray['comment'] ), 'native' );
                }
 
-               if ( $baseArray['xmp'] !== '' && function_exists( 'xml_parser_create_ns' ) ) {
+               if ( $baseArray['xmp'] !== '' && XMPReader::isSupported() ) {
                        $xmp = new XMPReader();
                        $xmp->parse( $baseArray['xmp'] );
                        $xmpRes = $xmp->getResults();
index 0d8013d..ae4af8d 100644 (file)
@@ -48,7 +48,7 @@ class JpegMetadataExtractor {
         * @throws MWException If given invalid file.
         */
        static function segmentSplitter( $filename ) {
-               $showXMP = function_exists( 'xml_parser_create_ns' );
+               $showXMP = XMPReader::isSupported();
 
                $segmentCount = 0;
 
index 0d341aa..50f04ae 100644 (file)
@@ -80,6 +80,12 @@ class XMPReader {
        /** @var int */
        private $extendedXMPOffset = 0;
 
+       /** @var int Flag determining if the XMP is safe to parse **/
+       private $parsable = 0;
+
+       /** @var string Buffer of XML to parse **/
+       private $xmlParsableBuffer = '';
+
        /**
         * These are various mode constants.
         * they are used to figure out what to do
@@ -108,6 +114,12 @@ class XMPReader {
        const NS_RDF = 'http://www.w3.org/1999/02/22-rdf-syntax-ns#';
        const NS_XML = 'http://www.w3.org/XML/1998/namespace';
 
+       // States used while determining if XML is safe to parse
+       const PARSABLE_UNKNOWN = 0;
+       const PARSABLE_OK = 1;
+       const PARSABLE_BUFFERING = 2;
+       const PARSABLE_NO = 3;
+
        /**
         * Constructor.
         *
@@ -145,6 +157,9 @@ class XMPReader {
                        array( $this, 'endElement' ) );
 
                xml_set_character_data_handler( $this->xmlParser, array( $this, 'char' ) );
+
+               $this->parsable = self::PARSABLE_UNKNOWN;
+               $this->xmlParsableBuffer = '';
        }
 
        /** Destroy the xml parser
@@ -156,6 +171,13 @@ class XMPReader {
                xml_parser_free( $this->xmlParser );
        }
 
+       /**
+        * Check if this instance supports using this class
+        */
+       public static function isSupported() {
+               return function_exists( 'xml_parser_create_ns' ) && class_exists( 'XMLReader' );
+       }
+
        /** Get the result array. Do some post-processing before returning
         * the array, and transform any metadata that is special-cased.
         *
@@ -305,6 +327,27 @@ class XMPReader {
                                wfRestoreWarnings();
                        }
 
+                       // Ensure the XMP block does not have an xml doctype declaration, which
+                       // could declare entities unsafe to parse with xml_parse (T85848/T71210).
+                       if ( $this->parsable !== self::PARSABLE_OK ) {
+                               if ( $this->parsable === self::PARSABLE_NO ) {
+                                       throw new Exception( 'Unsafe doctype declaration in XML.' );
+                               }
+
+                               $content = $this->xmlParsableBuffer . $content;
+                               if ( !$this->checkParseSafety( $content ) ) {
+                                       if ( !$allOfIt && $this->parsable !== self::PARSABLE_NO ) {
+                                               // parse wasn't Unsuccessful yet, so return true
+                                               // in this case.
+                                               return true;
+                                       }
+                                       $msg = ( $this->parsable === self::PARSABLE_NO ) ?
+                                               'Unsafe doctype declaration in XML.' :
+                                               'No root element found in XML.';
+                                       throw new Exception( $msg );
+                               }
+                       }
+
                        $ok = xml_parse( $this->xmlParser, $content, $allOfIt );
                        if ( !$ok ) {
                                $error = xml_error_string( xml_get_error_code( $this->xmlParser ) );
@@ -437,6 +480,62 @@ class XMPReader {
                }
        }
 
+       /**
+        * Check if a block of XML is safe to pass to xml_parse, i.e. doesn't
+        * contain a doctype declaration which could contain a dos attack if we
+        * parse it and expand internal entities (T85848).
+        *
+        * @param string $content xml string to check for parse safety
+        * @return bool true if the xml is safe to parse, false otherwise
+        */
+       private function checkParseSafety( $content ) {
+               $reader = new XMLReader();
+               $result = null;
+
+               // For XMLReader to parse incomplete/invalid XML, it has to be open()'ed
+               // instead of using XML().
+               $reader->open(
+                       'data://text/plain,' . urlencode( $content ),
+                       null,
+                       LIBXML_NOERROR | LIBXML_NOWARNING | LIBXML_NONET
+               );
+
+               $oldDisable = libxml_disable_entity_loader( true );
+               $reset = new ScopedCallback(
+                       'libxml_disable_entity_loader',
+                       array( $oldDisable )
+               );
+               $reader->setParserProperty( XMLReader::SUBST_ENTITIES, false );
+
+               // Even with LIBXML_NOWARNING set, XMLReader::read gives a warning
+               // when parsing truncated XML, which causes unit tests to fail.
+               wfSuppressWarnings();
+               while ( $reader->read() ) {
+                       if ( $reader->nodeType === XMLReader::ELEMENT ) {
+                               // Reached the first element without hitting a doctype declaration
+                               $this->parsable = self::PARSABLE_OK;
+                               $result = true;
+                               break;
+                       }
+                       if ( $reader->nodeType === XMLReader::DOC_TYPE ) {
+                               $this->parsable = self::PARSABLE_NO;
+                               $result = false;
+                               break;
+                       }
+               }
+               wfRestoreWarnings();
+
+               if ( !is_null( $result ) ) {
+                       return $result;
+               }
+
+               // Reached the end of the parsable xml without finding an element
+               // or doctype. Buffer and try again.
+               $this->parsable = self::PARSABLE_BUFFERING;
+               $this->xmlParsableBuffer = $content;
+               return false;
+       }
+
        /** When we hit a closing element in MODE_IGNORE
         * Check to see if this is the element we started to ignore,
         * in which case we get out of MODE_IGNORE
index 0452c41..d96e271 100644 (file)
@@ -3394,12 +3394,15 @@ class WikiPage implements Page, IDBAccessObject {
         * Opportunistically enqueue link update jobs given fresh parser output if useful
         *
         * @param ParserOutput $parserOutput Current version page output
-        * @return bool Whether a job was pushed
         * @since 1.25
         */
        public function triggerOpportunisticLinksUpdate( ParserOutput $parserOutput ) {
                if ( wfReadOnly() ) {
-                       return false;
+                       return;
+               }
+
+               if ( !Hooks::run( 'OpportunisticLinksUpdate', array( $this, $this->mTitle, $parserOutput ) ) ) {
+                       return;
                }
 
                if ( $this->mTitle->areRestrictionsCascading() ) {
@@ -3410,7 +3413,7 @@ class WikiPage implements Page, IDBAccessObject {
                        $params = array();
                } else {
                        // If the inclusions are deterministic, the edit-triggered link jobs are enough
-                       return false;
+                       return;
                }
 
                // Check if the last link refresh was before page_touched
@@ -3418,10 +3421,10 @@ class WikiPage implements Page, IDBAccessObject {
                        JobQueueGroup::singleton()->push( EnqueueJob::newFromLocalJobs(
                                new JobSpecification( 'refreshLinks', $params, array(), $this->mTitle )
                        ) );
-                       return true;
+                       return;
                }
 
-               return false;
+               return;
        }
 
        /**
index 20aac18..75ff8f3 100644 (file)
@@ -68,8 +68,9 @@ class LinkSearchPage extends QueryPage {
         * This allows for dependency injection even though we don't control object creation.
         */
        private function initServices() {
+               global $wgLanguageCode;
                if ( !$this->linkRenderer ) {
-                       $lang = $this->getContext()->getLanguage();
+                       $lang = Language::factory( $wgLanguageCode );
                        $titleFormatter = new MediaWikiTitleCodec( $lang, GenderCache::singleton() );
                        $this->linkRenderer = new MediaWikiPageLinkRenderer( $titleFormatter );
                }
index 44240d8..d6634a8 100644 (file)
@@ -545,14 +545,11 @@ class LoginForm extends SpecialPage {
                                return Status::newFatal( 'badretype' );
                        }
 
-                       # check for minimal password length
-                       $valid = $u->getPasswordValidity( $this->mPassword );
-                       if ( $valid !== true ) {
-                               if ( !is_array( $valid ) ) {
-                                       $valid = array( $valid, $wgMinimalPasswordLength );
-                               }
-
-                               return call_user_func_array( 'Status::newFatal', $valid );
+                       # check for password validity, return a fatal Status if invalid
+                       $validity = $u->checkPasswordValidity( $this->mPassword );
+                       if ( !$validity->isGood() ) {
+                               $validity->ok = false; // make sure this Status is fatal
+                               return $validity;
                        }
                }
 
index a79526e..6da8250 100644 (file)
@@ -1412,27 +1412,22 @@ abstract class UploadBase {
                                }
                        }
 
-                       # href with embedded svg as target
-                       if ( $stripped == 'href' && preg_match( '!data:[^,]*image/svg[^,]*,!sim', $value ) ) {
-                               wfDebug( __METHOD__ . ": Found href to embedded svg "
-                                       . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
-
-                               return true;
-                       }
-
-                       # href with embedded (text/xml) svg as target
-                       if ( $stripped == 'href' && preg_match( '!data:[^,]*text/xml[^,]*,!sim', $value ) ) {
-                               wfDebug( __METHOD__ . ": Found href to embedded svg "
-                                       . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
-
-                               return true;
+                       # only allow data: targets that should be safe. This prevents vectors like,
+                       # image/svg, text/xml, application/xml, and text/html, which can contain scripts
+                       if ( $stripped == 'href' && strncasecmp( 'data:', $value, 5 ) === 0 ) {
+                               // rfc2397 parameters. This is only slightly slower than (;[\w;]+)*.
+                               $parameters = '(?>;[a-zA-Z0-9\!#$&\'*+.^_`{|}~-]+=(?>[a-zA-Z0-9\!#$&\'*+.^_`{|}~-]+|"(?>[\0-\x0c\x0e-\x21\x23-\x5b\x5d-\x7f]+|\\\\[\0-\x7f])*"))*(?:;base64)?';
+                               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;
+                               }
                        }
 
-                       # Change href with animate from (http://html5sec.org/#137). This doesn't seem
-                       # possible without embedding the svg, but filter here in case.
-                       if ( $stripped == 'from'
+                       # Change href with animate from (http://html5sec.org/#137).
+                       if ( $stripped === 'attributename'
                                && $strippedElement === 'animate'
-                               && !preg_match( '!^https?://!im', $value )
+                               && $this->stripXmlNamespace( $value ) == 'href'
                        ) {
                                wfDebug( __METHOD__ . ": Found animate that might be changing href using from "
                                        . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" );
@@ -1524,7 +1519,7 @@ abstract class UploadBase {
        private static function checkCssFragment( $value ) {
 
                # Forbid external stylesheets, for both reliability and to protect viewer's privacy
-               if ( strpos( $value, '@import' ) !== false ) {
+               if ( stripos( $value, '@import' ) !== false ) {
                        return true;
                }
 
index f81567f..fb7056c 100644 (file)
        "wrongpassword": "Incorrect password entered.\nPlease try again.",
        "wrongpasswordempty": "Password entered was blank.\nPlease try again.",
        "passwordtooshort": "Passwords must be at least {{PLURAL:$1|1 character|$1 characters}}.",
+       "passwordtoolong": "Passwords cannot be longer than {{PLURAL:$1|1 character|$1 characters}}.",
        "password-name-match": "Your password must be different from your username.",
        "password-login-forbidden": "The use of this username and password has been forbidden.",
        "mailmypassword": "Reset password",
index fef9ac8..b7c31fc 100644 (file)
        "wrongpassword": "Used as error message when the provided password is wrong.\nThis message is used in html.\n{{Identical|Please try again}}",
        "wrongpasswordempty": "Error message displayed when entering a blank password.\n{{Identical|Please try again}}",
        "passwordtooshort": "This message is shown in [[Special:Preferences]] and [[Special:CreateAccount]].\n\nParameters:\n* $1 - the minimum number of characters in the password",
+       "passwordtoolong": "This message is shown in [[Special:Preferences]], [[Special:CreateAccount]], and [[Special:Userlogin]].\n\nParameters:\n* $1 - the maximum number of characters in the password",
        "password-name-match": "Used as error message when password validity check failed.",
        "password-login-forbidden": "Error message shown when the user has tried to log in using one of the special username/password combinations used for MediaWiki testing. (See [[mwr:75589]], [[mwr:75605]].)",
        "mailmypassword": "Used as label for Submit button in [[Special:PasswordReset]].\n{{Identical|Reset password}}",
index 6075edf..f9d2eac 100644 (file)
@@ -1709,14 +1709,6 @@ return array(
                'targets' => array( 'desktop', 'mobile' ),
        ),
 
-       'oojs-ui.styles' => array(
-               'position' => 'top',
-               'skinStyles' => array(
-                       'default' => 'resources/lib/oojs-ui/oojs-ui-mediawiki.css',
-               ),
-               'targets' => array( 'desktop', 'mobile' ),
-       ),
-
        // FIXME: This is a bit of a mess; T92551 requests fixing
        'oojs-ui.styles.icons-alerts' => array(
                'position' => 'top',
index 2b7f4cd..f660678 100644 (file)
@@ -13901,7 +13901,7 @@ section 5
 </ul>
 </div>
 
-<h2><span class="mw-headline" id="text_.3E_text">text &gt; text</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=1" title="Edit section: text > text">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
+<h2><span class="mw-headline" id="text_.3E_text">text &gt; text</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=1" title="Edit section: text &gt; text">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
 <p>section 1
 </p>
 <h2><span class="mw-headline" id="text_.3C_text">text &lt; text</span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=2" title="Edit section: text &lt; text">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
@@ -19608,7 +19608,7 @@ __TOC__
 </div>
 
 <h2><span class="mw-headline" id="Hello"><sup class="in-h2">Hello</sup></span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=1" title="Edit section: Hello">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
-<h2><span class="mw-headline" id="b.22.3EEvilbye"><sup> b"&gt;Evilbye</sup></span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=2" title="Edit section: b&quot;>Evilbye">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
+<h2><span class="mw-headline" id="b.22.3EEvilbye"><sup> b"&gt;Evilbye</sup></span><span class="mw-editsection"><span class="mw-editsection-bracket">[</span><a href="/index.php?title=Parser_test&amp;action=edit&amp;section=2" title="Edit section: b&quot;&gt;Evilbye">edit</a><span class="mw-editsection-bracket">]</span></span></h2>
 
 !! end
 
diff --git a/tests/phpunit/data/xmp/doctype-included.result.php b/tests/phpunit/data/xmp/doctype-included.result.php
new file mode 100644 (file)
index 0000000..9a9cc36
--- /dev/null
@@ -0,0 +1,3 @@
+<?php
+
+$result = array();
diff --git a/tests/phpunit/data/xmp/doctype-included.xmp b/tests/phpunit/data/xmp/doctype-included.xmp
new file mode 100644 (file)
index 0000000..8c94675
--- /dev/null
@@ -0,0 +1,12 @@
+<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?> <!DOCTYPE x:xmpmeta [ <!ENTITY lol "lollollollollollollollollollollol"> ]>
+<x:xmpmeta xmlns:x="adobe:ns:meta/" x:xmptk="Adobe XMP Core
+ 4.1.3-c001 49.282696, Mon Apr 02 2007 21:16:10        ">
+<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
+<rdf:Description
+ rdf:about=""
+ xmlns:exif="http://ns.adobe.com/exif/1.0/"
+ exif:DigitalZoomRatio="0/10">
+<exif:Flash rdf:parseType='Resource'>
+<exif:Fired>True</exif:Fired> <exif:Return>0</exif:Return> <exif:Mode>1</exif:Mode> <exif:Function>False</exif:Function> <exif:RedEyeMode>False</exif:RedEyeMode></exif:Flash> </rdf:Description> </rdf:RDF> </x:xmpmeta>
+
+<?xpacket end="w"?>
diff --git a/tests/phpunit/data/xmp/doctype-not-included.xmp b/tests/phpunit/data/xmp/doctype-not-included.xmp
new file mode 100644 (file)
index 0000000..9a40b4b
--- /dev/null
@@ -0,0 +1,11 @@
+<?xpacket begin="" id="W5M0MpCehiHzreSzNTczkc9d"?> <x:xmpmeta xmlns:x="adobe:ns:meta/" x:xmptk="Adobe XMP Core
+ 4.1.3-c001 49.282696, Mon Apr 02 2007 21:16:10        ">
+<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
+<rdf:Description
+ rdf:about=""
+ xmlns:exif="http://ns.adobe.com/exif/1.0/"
+ exif:DigitalZoomRatio="0/10">
+<exif:Flash rdf:parseType='Resource'>
+<exif:Fired>True</exif:Fired> <exif:Return>0</exif:Return> <exif:Mode>1</exif:Mode> <exif:Function>False</exif:Function> <exif:RedEyeMode>False</exif:RedEyeMode></exif:Flash> </rdf:Description> </rdf:RDF> </x:xmpmeta>
+
+<?xpacket end="w"?>
index 860529e..73e5051 100644 (file)
@@ -306,7 +306,10 @@ class UserTest extends MediaWikiTestCase {
         * @covers User::isValidPassword()
         */
        public function testCheckPasswordValidity() {
-               $this->setMwGlobals( 'wgMinimalPasswordLength', 6 );
+               $this->setMwGlobals( array(
+                       'wgMinimalPasswordLength' => 6,
+                       'wgMaximalPasswordLength' => 30,
+               ) );
                $user = User::newFromName( 'Useruser' );
                // Sanity
                $this->assertTrue( $user->isValidPassword( 'Password1234' ) );
@@ -314,10 +317,19 @@ class UserTest extends MediaWikiTestCase {
                // Minimum length
                $this->assertFalse( $user->isValidPassword( 'a' ) );
                $this->assertFalse( $user->checkPasswordValidity( 'a' )->isGood() );
+               $this->assertTrue( $user->checkPasswordValidity( 'a' )->isOK() );
                $this->assertEquals( 'passwordtooshort', $user->getPasswordValidity( 'a' ) );
 
+               // Maximum length
+               $longPass = str_repeat( 'a', 31 );
+               $this->assertFalse( $user->isValidPassword( $longPass ) );
+               $this->assertFalse( $user->checkPasswordValidity( $longPass )->isGood() );
+               $this->assertFalse( $user->checkPasswordValidity( $longPass )->isOK() );
+               $this->assertEquals( 'passwordtoolong', $user->getPasswordValidity( $longPass ) );
+
                // Matches username
                $this->assertFalse( $user->checkPasswordValidity( 'Useruser' )->isGood() );
+               $this->assertTrue( $user->checkPasswordValidity( 'Useruser' )->isOK() );
                $this->assertEquals( 'password-name-match', $user->getPasswordValidity( 'Useruser' ) );
 
                // On the forbidden list
index e7b3e77..f0ba934 100644 (file)
@@ -8,6 +8,7 @@
 class XmlTypeCheckTest extends PHPUnit_Framework_TestCase {
        const WELL_FORMED_XML = "<root><child /></root>";
        const MAL_FORMED_XML = "<root><child /></error>";
+       const XML_WITH_PIH = '<?xml version="1.0"?><?xml-stylesheet type="text/xsl" href="/w/index.php"?><svg><child /></svg>';
 
        /**
         * @covers XMLTypeCheck::newFromString
@@ -27,4 +28,22 @@ class XmlTypeCheckTest extends PHPUnit_Framework_TestCase {
                $this->assertFalse( $testXML->wellFormed );
        }
 
+       /**
+        * @covers XMLTypeCheck::processingInstructionHandler
+        */
+       public function testProcessingInstructionHandler() {
+               $called = false;
+               $testXML = new XmlTypeCheck(
+                       self::XML_WITH_PIH,
+                       null,
+                       false,
+                       array(
+                               'processing_instruction_handler' => function() use ( &$called ) {
+                                       $called = true;
+                               }
+                       )
+               );
+               $this->assertTrue( $called );
+       }
+
 }
index 798e492..6758e94 100644 (file)
@@ -62,6 +62,8 @@ class XMPTest extends MediaWikiTestCase {
                        array( 'gps', 'Handling of exif GPS parameters in XMP' ),
                );
 
+               $xmpFiles[] = array( 'doctype-included', 'XMP includes doctype' );
+
                foreach ( $xmpFiles as $file ) {
                        $xmp = file_get_contents( $xmpPath . $file[0] . '.xmp' );
                        // I'm not sure if this is the best way to handle getting the
@@ -170,4 +172,52 @@ class XMPTest extends MediaWikiTestCase {
 
                $this->assertEquals( $expected, $actual );
        }
+
+       /**
+        * Test for multi-section, hostile XML
+        * @covers checkParseSafety
+        */
+       public function testCheckParseSafety() {
+
+               // Test for detection
+               $xmpPath = __DIR__ . '/../../data/xmp/';
+               $file = fopen( $xmpPath . 'doctype-included.xmp', 'rb' );
+               $valid = false;
+               $reader = new XMPReader();
+               do {
+                       $chunk = fread( $file, 10 );
+                       $valid = $reader->parse( $chunk, feof( $file ) );
+               } while ( !feof( $file ) );
+               $this->assertFalse( $valid, 'Check that doctype is detected in fragmented XML' );
+               $this->assertEquals(
+                       array(),
+                       $reader->getResults(),
+                       'Check that doctype is detected in fragmented XML'
+               );
+               fclose( $file );
+               unset( $reader );
+
+               // Test for false positives
+               $file = fopen( $xmpPath . 'doctype-not-included.xmp', 'rb' );
+               $valid = false;
+               $reader = new XMPReader();
+               do {
+                       $chunk = fread( $file, 10 );
+                       $valid = $reader->parse( $chunk, feof( $file ) );
+               } while ( !feof( $file ) );
+               $this->assertTrue(
+                       $valid,
+                       'Check for false-positive detecting doctype in fragmented XML'
+               );
+               $this->assertEquals(
+                       array(
+                               'xmp-exif' => array(
+                                       'DigitalZoomRatio' => '0/10',
+                                       'Flash' => '9'
+                               )
+                       ),
+                       $reader->getResults(),
+                       'Check that doctype is detected in fragmented XML'
+               );
+       }
 }
index 63ad8c0..9441b77 100644 (file)
@@ -162,6 +162,12 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                'SVG with javascript xlink (http://html5sec.org/#87)'
                        ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><use xlink:href="data:application/xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHhtbG5zOnhsaW5rPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rIj4KPGRlZnM+CjxjaXJjbGUgaWQ9InRlc3QiIHI9IjUwIiBjeD0iMTAwIiBjeT0iMTAwIiBzdHlsZT0iZmlsbDogI0YwMCI+CjxzZXQgYXR0cmlidXRlTmFtZT0iZmlsbCIgYXR0cmlidXRlVHlwZT0iQ1NTIiBvbmJlZ2luPSdhbGVydChkb2N1bWVudC5jb29raWUpJwpvbmVuZD0nYWxlcnQoIm9uZW5kIiknIHRvPSIjMDBGIiBiZWdpbj0iMXMiIGR1cj0iNXMiIC8+CjwvY2lyY2xlPgo8L2RlZnM+Cjx1c2UgeGxpbms6aHJlZj0iI3Rlc3QiLz4KPC9zdmc+#test"/> </svg>',
+                               true,
+                               true,
+                               'SVG with Opera image xlink (http://html5sec.org/#88 - c)'
+                       ),
                        array(
                                '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">  <animation xlink:href="javascript:alert(1)"/> </svg>',
                                true,
@@ -273,6 +279,18 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                'SVG with animate from (http://html5sec.org/#137)'
                        ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> <a><text y="1em">Click me</text> <animate attributeName="xlink:href" values="javascript:alert(\'Bang!\')" begin="0s" dur="0.1s" fill="freeze" /> </a></svg>',
+                               true,
+                               true,
+                               'SVG with animate xlink:href (http://html5sec.org/#137)'
+                       ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" xmlns:y="http://www.w3.org/1999/xlink"> <a y:href="#"> <text y="1em">Click me</text> <animate attributeName="y:href" values="javascript:alert(\'Bang!\')" begin="0s" dur="0.1s" fill="freeze" /> </a> </svg>',
+                               true,
+                               true,
+                               'SVG with animate y:href (http://html5sec.org/#137)'
+                       ),
 
                        // Other hostile SVG's
                        array(
@@ -305,6 +323,12 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                'SVG with @import in style element and child element (bug 69008#c11)'
                        ),
+                       array(
+                               '<svg xmlns="http://www.w3.org/2000/svg" viewBox="6 3 177 153" xmlns:xlink="http://www.w3.org/1999/xlink"> <style>@imporT "https://fonts.googleapis.com/css?family=Bitter:700&amp;text=WebPlatform.org";</style> <g transform="translate(-.5,-.5)"> <text fill="#474747" x="95" y="150" text-anchor="middle" font-family="Bitter" font-size="20" font-weight="bold">WebPlatform.org</text> </g> </svg>',
+                               true,
+                               true,
+                               'SVG with case-insensitive @import in style element (bug T85349)'
+                       ),
                        array(
                                '<svg xmlns="http://www.w3.org/2000/svg"> <rect width="100" height="100" style="background-image:url(https://www.google.com/images/srpr/logo11w.png)"/> </svg>',
                                true,
@@ -331,6 +355,25 @@ class UploadBaseTest extends MediaWikiTestCase {
                                true,
                                'SVG with remote background image using image() (bug 69008)'
                        ),
+                       array(
+                               // As reported by Cure53
+                               '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> <a xlink:href="data:text/html;charset=utf-8;base64, PHNjcmlwdD5hbGVydChkb2N1bWVudC5kb21haW4pPC9zY3JpcHQ%2BDQo%3D"> <circle r="400" fill="red"></circle> </a> </svg>',
+                               true,
+                               true,
+                               'SVG with data:text/html link target (firefox only)'
+                       ),
+                       array(
+                               '<?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,
+                               true,
+                               'SVG with encoded script tag in internal entity (reported by Beyond Security)'
+                       ),
+                       array(
+                               '<?xml version="1.0"?> <!DOCTYPE svg [ <!ENTITY foo SYSTEM "file:///etc/passwd"> ]> <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,
+                               'SVG with external entity'
+                       ),
 
                        // Test good, but strange files that we want to allow
                        array(
@@ -345,7 +388,6 @@ class UploadBaseTest extends MediaWikiTestCase {
                                false,
                                'SVG with local urls, including filter: in style'
                        ),
-
                );
        }
 }