diff options
Diffstat (limited to 'includes')
-rw-r--r-- | includes/DefaultSettings.php | 14 | ||||
-rw-r--r-- | includes/EditPage.php | 26 | ||||
-rw-r--r-- | includes/Html.php | 7 | ||||
-rw-r--r-- | includes/OutputPage.php | 8 | ||||
-rw-r--r-- | includes/User.php | 40 | ||||
-rw-r--r-- | includes/Xml.php | 4 | ||||
-rw-r--r-- | includes/api/ApiFormatWddx.php | 48 | ||||
-rw-r--r-- | includes/installer/PostgresUpdater.php | 24 | ||||
-rw-r--r-- | includes/libs/XmlTypeCheck.php | 251 | ||||
-rw-r--r-- | includes/media/BitmapMetadataHandler.php | 6 | ||||
-rw-r--r-- | includes/media/JpegMetadataExtractor.php | 2 | ||||
-rw-r--r-- | includes/media/XMP.php | 96 | ||||
-rw-r--r-- | includes/specialpage/SpecialPageFactory.php | 4 | ||||
-rw-r--r-- | includes/specials/SpecialActiveusers.php | 8 | ||||
-rw-r--r-- | includes/specials/SpecialJavaScriptTest.php | 248 | ||||
-rw-r--r-- | includes/specials/SpecialUserlogin.php | 13 | ||||
-rw-r--r-- | includes/upload/UploadBase.php | 33 |
17 files changed, 589 insertions, 243 deletions
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 71268932..aad42aac 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -75,7 +75,7 @@ $wgConfigRegistry = array( * Using single quotes is, therefore, important here. * @since 1.2 */ -$wgVersion = '1.24.1'; +$wgVersion = '1.24.2'; /** * Name of the site. It must be changed in LocalSettings.php @@ -4146,6 +4146,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(). * @since 1.23 diff --git a/includes/EditPage.php b/includes/EditPage.php index 128244a8..38c80ba8 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -2654,19 +2654,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' ) + ); + } } } } diff --git a/includes/Html.php b/includes/Html.php index 1e16e394..2e148140 100644 --- a/includes/Html.php +++ b/includes/Html.php @@ -546,17 +546,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( '&' => '&', '"' => '"', + '>' => '>', "\n" => ' ', "\r" => ' ', "\t" => '	' diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 2f8094ab..55b1da00 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2743,7 +2743,7 @@ $templates * call rather than a "<script src='...'>" tag. * @return string The html "<script>", "<link>" and "<style>" tags */ - protected function makeResourceLoaderLink( $modules, $only, $useESI = false, + public function makeResourceLoaderLink( $modules, $only, $useESI = false, array $extraQuery = array(), $loadCall = false ) { $modules = (array)$modules; @@ -3153,7 +3153,7 @@ $templates * have to be purged on configuration changes. * @return array */ - private function getJSVars() { + public function getJSVars() { global $wgContLang; $curRevisionId = 0; @@ -3289,6 +3289,10 @@ $templates 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() ) ); } diff --git a/includes/User.php b/includes/User.php index 5e5d3eed..a925a3c4 100644 --- a/includes/User.php +++ b/includes/User.php @@ -773,15 +773,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 @@ -801,6 +810,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; @@ -2300,17 +2313,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() ); } } @@ -3792,6 +3797,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). diff --git a/includes/Xml.php b/includes/Xml.php index 159f7114..c6c02867 100644 --- a/includes/Xml.php +++ b/includes/Xml.php @@ -707,13 +707,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 diff --git a/includes/api/ApiFormatWddx.php b/includes/api/ApiFormatWddx.php index ba90c260..ec3dc2d9 100644 --- a/includes/api/ApiFormatWddx.php +++ b/includes/api/ApiFormatWddx.php @@ -38,15 +38,7 @@ class ApiFormatWddx extends ApiFormatBase { public function execute() { $this->markDeprecated(); - // Some versions of PHP have a broken wddx_serialize_value, see - // PHP bug 45314. Test encoding an affected character (U+00A0) - // to avoid this. - $expected = - "<wddxPacket version='1.0'><header/><data><string>\xc2\xa0</string></data></wddxPacket>"; - if ( function_exists( 'wddx_serialize_value' ) - && !$this->getIsHtml() - && wddx_serialize_value( "\xc2\xa0" ) == $expected - ) { + if ( !$this->getIsHtml() && !static::useSlowPrinter() ) { $this->printText( wddx_serialize_value( $this->getResultData() ) ); } else { // Don't do newlines and indentation if we weren't asked @@ -63,6 +55,44 @@ class ApiFormatWddx extends ApiFormatBase { } } + public static function useSlowPrinter() { + if ( !function_exists( 'wddx_serialize_value' ) ) { + return true; + } + + // Some versions of PHP have a broken wddx_serialize_value, see + // PHP bug 45314. Test encoding an affected character (U+00A0) + // to avoid this. + $expected = + "<wddxPacket version='1.0'><header/><data><string>\xc2\xa0</string></data></wddxPacket>"; + if ( wddx_serialize_value( "\xc2\xa0" ) !== $expected ) { + return true; + } + + // Some versions of HHVM don't correctly encode ampersands. + $expected = + "<wddxPacket version='1.0'><header/><data><string>&</string></data></wddxPacket>"; + if ( wddx_serialize_value( '&' ) !== $expected ) { + return true; + } + + // Some versions of HHVM don't correctly encode empty arrays as subvalues. + $expected = + "<wddxPacket version='1.0'><header/><data><array length='1'><array length='0'></array></array></data></wddxPacket>"; + if ( wddx_serialize_value( array( array() ) ) !== $expected ) { + return true; + } + + // Some versions of HHVM don't correctly encode associative arrays with numeric keys. + $expected = + "<wddxPacket version='1.0'><header/><data><struct><var name='2'><number>1</number></var></struct></data></wddxPacket>"; + if ( wddx_serialize_value( array( 2 => 1 ) ) !== $expected ) { + return true; + } + + return false; + } + /** * Recursively go through the object and output its data in WDDX format. * @param mixed $elemValue diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index 9e8ee94f..df2f0e32 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -384,8 +384,6 @@ class PostgresUpdater extends DatabaseUpdater { 'page(page_id) ON DELETE CASCADE' ), array( 'changeFkeyDeferrable', 'protected_titles', 'pt_user', 'mwuser(user_id) ON DELETE SET NULL' ), - array( 'changeFkeyDeferrable', 'recentchanges', 'rc_cur_id', - 'page(page_id) ON DELETE SET NULL' ), array( 'changeFkeyDeferrable', 'recentchanges', 'rc_user', 'mwuser(user_id) ON DELETE SET NULL' ), array( 'changeFkeyDeferrable', 'redirect', 'rd_from', 'page(page_id) ON DELETE CASCADE' ), @@ -418,6 +416,10 @@ class PostgresUpdater extends DatabaseUpdater { array( 'addPgField', 'pagelinks', 'pl_from_namespace', 'INTEGER NOT NULL DEFAULT 0' ), array( 'addPgField', 'templatelinks', 'tl_from_namespace', 'INTEGER NOT NULL DEFAULT 0' ), array( 'addPgField', 'imagelinks', 'il_from_namespace', 'INTEGER NOT NULL DEFAULT 0' ), + + // 1.24.1 (backport from 1.25) + array( 'dropFkey', 'recentchanges', 'rc_cur_id' ) + ); } @@ -769,6 +771,24 @@ END; } } + protected function dropFkey( $table, $field ) { + $fi = $this->db->fieldInfo( $table, $field ); + if ( is_null( $fi ) ) { + $this->output( "WARNING! Column '$table.$field' does not exist but it should! " . + "Please report this.\n" ); + return; + } + $conname = $fi->conname(); + if ( $fi->conname() ) { + $this->output( "Dropping foreign key constraint on '$table.$field'\n" ); + $conclause = "CONSTRAINT \"$conname\""; + $command = "ALTER TABLE $table DROP CONSTRAINT $conname"; + $this->db->query( $command ); + } else { + $this->output( "...foreign key constraint on '$table.$field' already does not exist\n" ); + }; + } + protected function changeFkeyDeferrable( $table, $field, $clause ) { $fi = $this->db->fieldInfo( $table, $field ); if ( is_null( $fi ) ) { diff --git a/includes/libs/XmlTypeCheck.php b/includes/libs/XmlTypeCheck.php index aca857e9..31a4e28a 100644 --- a/includes/libs/XmlTypeCheck.php +++ b/includes/libs/XmlTypeCheck.php @@ -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; + } } } } diff --git a/includes/media/BitmapMetadataHandler.php b/includes/media/BitmapMetadataHandler.php index dd41c388..1d790155 100644 --- a/includes/media/BitmapMetadataHandler.php +++ b/includes/media/BitmapMetadataHandler.php @@ -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(); diff --git a/includes/media/JpegMetadataExtractor.php b/includes/media/JpegMetadataExtractor.php index 8c5b46bb..aaa9930a 100644 --- a/includes/media/JpegMetadataExtractor.php +++ b/includes/media/JpegMetadataExtractor.php @@ -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; diff --git a/includes/media/XMP.php b/includes/media/XMP.php index cdbd5ab2..a3f45e6c 100644 --- a/includes/media/XMP.php +++ b/includes/media/XMP.php @@ -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 MWException( '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 MWException( $msg ); + } + } + $ok = xml_parse( $this->xmlParser, $content, $allOfIt ); if ( !$ok ) { $error = xml_error_string( xml_get_error_code( $this->xmlParser ) ); @@ -437,6 +480,59 @@ 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 ); + $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(); + libxml_disable_entity_loader( $oldDisable ); + + 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 diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php index 679492ae..23fdc71a 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -74,7 +74,7 @@ class SpecialPageFactory { 'Wantedtemplates' => 'WantedTemplatesPage', // List of pages - 'Allpages' => 'SpecialAllpages', + 'Allpages' => 'SpecialAllPages', 'Prefixindex' => 'SpecialPrefixindex', 'Categories' => 'SpecialCategories', 'Listredirects' => 'ListredirectsPage', @@ -123,7 +123,7 @@ class SpecialPageFactory { // Data and tools 'Statistics' => 'SpecialStatistics', - 'Allmessages' => 'SpecialAllmessages', + 'Allmessages' => 'SpecialAllMessages', 'Version' => 'SpecialVersion', 'Lockdb' => 'SpecialLockdb', 'Unlockdb' => 'SpecialUnlockdb', diff --git a/includes/specials/SpecialActiveusers.php b/includes/specials/SpecialActiveusers.php index ce436525..f983b452 100644 --- a/includes/specials/SpecialActiveusers.php +++ b/includes/specials/SpecialActiveusers.php @@ -115,10 +115,16 @@ class ActiveUsersPager extends UsersPager { ) . ')'; } + if ( $dbr->implicitGroupby() ) { + $options = array( 'GROUP BY' => array( 'qcc_title' ) ); + } else { + $options = array( 'GROUP BY' => array( 'user_name', 'user_id', 'qcc_title' ) ); + } + return array( 'tables' => array( 'querycachetwo', 'user', 'recentchanges' ), 'fields' => array( 'user_name', 'user_id', 'recentedits' => 'COUNT(*)', 'qcc_title' ), - 'options' => array( 'GROUP BY' => array( 'qcc_title' ) ), + 'options' => $options, 'conds' => $conds ); } diff --git a/includes/specials/SpecialJavaScriptTest.php b/includes/specials/SpecialJavaScriptTest.php index 0efebb3e..7d745a50 100644 --- a/includes/specials/SpecialJavaScriptTest.php +++ b/includes/specials/SpecialJavaScriptTest.php @@ -26,12 +26,10 @@ */ class SpecialJavaScriptTest extends SpecialPage { /** - * @var array Mapping of framework ids and their initilizer methods - * in this class. If a framework is requested but not in this array, - * the 'unknownframework' error is served. + * @var array Supported frameworks. */ private static $frameworks = array( - 'qunit' => 'initQUnitTesting', + 'qunit', ); public function __construct() { @@ -44,43 +42,70 @@ class SpecialJavaScriptTest extends SpecialPage { $this->setHeaders(); $out->disallowUserJs(); - $out->addModules( 'mediawiki.special.javaScriptTest' ); - - // Determine framework - $pars = explode( '/', $par ); - $framework = strtolower( $pars[0] ); - - // No framework specified - if ( $par == '' ) { + if ( $par === null ) { + // No framework specified + $out->setStatusCode( 404 ); $out->setPageTitle( $this->msg( 'javascripttest' ) ); - $summary = $this->wrapSummaryHtml( - $this->msg( 'javascripttest-pagetext-noframework' )->escaped() . - $this->getFrameworkListHtml(), - 'noframework' + $out->addHTML( + $this->msg( 'javascripttest-pagetext-noframework' )->parseAsBlock() + . $this->getFrameworkListHtml() ); - $out->addHtml( $summary ); - } elseif ( isset( self::$frameworks[$framework] ) ) { - // Matched! Display proper title and initialize the framework - $out->setPageTitle( $this->msg( - 'javascripttest-title', - // Messages: javascripttest-qunit-name - $this->msg( "javascripttest-$framework-name" )->plain() - ) ); - $out->setSubtitle( $this->msg( 'javascripttest-backlink' ) - ->rawParams( Linker::linkKnown( $this->getPageTitle() ) ) ); - $this->{self::$frameworks[$framework]}(); - } else { - // Framework not found, display error - $out->setPageTitle( $this->msg( 'javascripttest' ) ); - $summary = $this->wrapSummaryHtml( - '<p class="error">' . - $this->msg( 'javascripttest-pagetext-unknownframework', $par )->escaped() . - '</p>' . - $this->getFrameworkListHtml(), - 'unknownframework' + return; + } + + // Determine framework and mode + $pars = explode( '/', $par, 2 ); + + $framework = $pars[0]; + if ( !in_array( $framework, self::$frameworks ) ) { + // Framework not found + $out->setStatusCode( 404 ); + $out->addHTML( + '<div class="error">' + . $this->msg( 'javascripttest-pagetext-unknownframework' ) + ->plaintextParams( $par )->parseAsBlock() + . '</div>' + . $this->getFrameworkListHtml() ); - $out->addHtml( $summary ); + return; } + + // This special page is disabled by default ($wgEnableJavaScriptTest), and contains + // no sensitive data. In order to allow TestSwarm to embed it into a test client window, + // we need to allow iframing of this page. + $out->allowClickjacking(); + $out->setSubtitle( + $this->msg( 'javascripttest-backlink' ) + ->rawParams( Linker::linkKnown( $this->getPageTitle() ) ) + ); + + // Custom actions + if ( isset( $pars[1] ) ) { + $action = $pars[1]; + if ( !in_array( $action, array( 'export', 'plain' ) ) ) { + $out->setStatusCode( 404 ); + $out->addHTML( + '<div class="error">' + . $this->msg( 'javascripttest-pagetext-unknownaction' ) + ->plaintextParams( $action )->parseAsBlock() + . '</div>' + ); + return; + } + $method = $action . ucfirst( $framework ); + $this->$method(); + return; + } + + $out->addModules( 'mediawiki.special.javaScriptTest' ); + + $method = 'view' . ucfirst( $framework ); + $this->$method(); + $out->setPageTitle( $this->msg( + 'javascripttest-title', + // Messages: javascripttest-qunit-name + $this->msg( "javascripttest-$framework-name" )->plain() + ) ); } /** @@ -91,7 +116,7 @@ class SpecialJavaScriptTest extends SpecialPage { */ private function getFrameworkListHtml() { $list = '<ul>'; - foreach ( self::$frameworks as $framework => $initFn ) { + foreach ( self::$frameworks as $framework ) { $list .= Html::rawElement( 'li', array(), @@ -109,60 +134,35 @@ class SpecialJavaScriptTest extends SpecialPage { } /** - * Function to wrap the summary. - * It must be given a valid state as a second parameter or an exception will - * be thrown. - * @param string $html The raw HTML. - * @param string $state State, one of 'noframework', 'unknownframework' or 'frameworkfound' - * @throws MWException - * @return string + * Wrap HTML contents in a summary container. + * + * @param string $html HTML contents to be wrapped + * @return string HTML */ - private function wrapSummaryHtml( $html, $state ) { - $validStates = array( 'noframework', 'unknownframework', 'frameworkfound' ); - - if ( !in_array( $state, $validStates ) ) { - throw new MWException( __METHOD__ - . ' given an invalid state. Must be one of "' - . join( '", "', $validStates ) . '".' - ); - } - - return "<div id=\"mw-javascripttest-summary\" class=\"mw-javascripttest-$state\">$html</div>"; + private function wrapSummaryHtml( $html ) { + return "<div id=\"mw-javascripttest-summary\">$html</div>"; } /** - * Initialize the page for QUnit. + * Run the test suite on the Special page. + * + * Rendered by OutputPage and Skin. */ - private function initQUnitTesting() { + private function viewQUnit() { $out = $this->getOutput(); $testConfig = $this->getConfig()->get( 'JavaScriptTestConfig' ); - $out->addModules( 'test.mediawiki.qunit.testrunner' ); - $qunitTestModules = $out->getResourceLoader()->getTestModuleNames( 'qunit' ); - $out->addModules( $qunitTestModules ); + $modules = $out->getResourceLoader()->getTestModuleNames( 'qunit' ); $summary = $this->msg( 'javascripttest-qunit-intro' ) ->params( $testConfig['qunit']['documentation'] ) ->parseAsBlock(); - $header = $this->msg( 'javascripttest-qunit-heading' )->escaped(); - $userDir = $this->getLanguage()->getDir(); $baseHtml = <<<HTML <div class="mw-content-ltr"> -<div id="qunit-header"><span dir="$userDir">$header</span></div> -<div id="qunit-banner"></div> -<div id="qunit-testrunner-toolbar"></div> -<div id="qunit-userAgent"></div> -<ol id="qunit-tests"></ol> -<div id="qunit-fixture">test markup, will be hidden</div> +<div id="qunit"></div> </div> HTML; - $out->addHtml( $this->wrapSummaryHtml( $summary, 'frameworkfound' ) . $baseHtml ); - - // This special page is disabled by default ($wgEnableJavaScriptTest), and contains - // no sensitive data. In order to allow TestSwarm to embed it into a test client window, - // we need to allow iframing of this page. - $out->allowClickjacking(); // Used in ./tests/qunit/data/testrunner.js, see also documentation of // $wgJavaScriptTestConfig in DefaultSettings.php @@ -170,6 +170,102 @@ HTML; 'QUnitTestSwarmInjectJSPath', $testConfig['qunit']['testswarm-injectjs'] ); + + $out->addHtml( $this->wrapSummaryHtml( $summary ) . $baseHtml ); + + // The testrunner configures QUnit and essentially depends on it. However, test suites + // are reusable in environments that preload QUnit (or a compatibility interface to + // another framework). Therefore we have to load it ourselves. + $out->addHtml( Html::inlineScript( + ResourceLoader::makeLoaderConditionalScript( + Xml::encodeJsCall( 'mw.loader.using', array( + array( 'jquery.qunit', 'jquery.qunit.completenessTest' ), + new XmlJsCode( + 'function () {' . Xml::encodeJsCall( 'mw.loader.load', array( $modules ) ) . '}' + ) + ) ) + ) + ) ); + } + + /** + * Generate self-sufficient JavaScript payload to run the tests elsewhere. + * + * Includes startup module to request modules from ResourceLoader. + * + * Note: This modifies the registry to replace 'jquery.qunit' with an + * empty module to allow external environment to preload QUnit with any + * neccecary framework adapters (e.g. Karma). Loading it again would + * re-define QUnit and dereference event handlers from Karma. + */ + private function exportQUnit() { + $out = $this->getOutput(); + + $out->disable(); + + $rl = $out->getResourceLoader(); + + $query = array( + 'lang' => $this->getLanguage()->getCode(), + 'skin' => $this->getSkin()->getSkinName(), + 'debug' => ResourceLoader::inDebugMode() ? 'true' : 'false', + ); + $embedContext = new ResourceLoaderContext( $rl, new FauxRequest( $query ) ); + $query['only'] = 'scripts'; + $startupContext = new ResourceLoaderContext( $rl, new FauxRequest( $query ) ); + + $modules = $rl->getTestModuleNames( 'qunit' ); + + // The below is essentially a pure-javascript version of OutputPage::getHeadScripts. + $startup = $rl->makeModuleResponse( $startupContext, array( + 'startup' => $rl->getModule( 'startup' ), + ) ); + // Embed page-specific mw.config variables. + // The current Special page shouldn't be relevant to tests, but various modules (which + // are loaded before the test suites), reference mw.config while initialising. + $code = ResourceLoader::makeConfigSetScript( $out->getJSVars() ); + // Embed private modules as they're not allowed to be loaded dynamically + $code .= $rl->makeModuleResponse( $embedContext, array( + 'user.options' => $rl->getModule( 'user.options' ), + 'user.tokens' => $rl->getModule( 'user.tokens' ), + ) ); + $code .= Xml::encodeJsCall( 'mw.loader.load', array( $modules ) ); + + header( 'Content-Type: text/javascript; charset=utf-8' ); + header( 'Cache-Control: private, no-cache, must-revalidate' ); + header( 'Pragma: no-cache' ); + echo $startup; + echo "\n"; + // Note: The following has to be wrapped in a script tag because the startup module also + // writes a script tag (the one loading mediawiki.js). Script tags are synchronous, block + // each other, and run in order. But they don't nest. The code appended after the startup + // module runs before the added script tag is parsed and executed. + echo Xml::encodeJsCall( 'document.write', array( Html::inlineScript( $code ) ) ); + } + + private function plainQUnit() { + $out = $this->getOutput(); + $out->disable(); + + $url = $this->getPageTitle( 'qunit/export' )->getFullURL( array( + 'debug' => ResourceLoader::inDebugMode() ? 'true' : 'false', + ) ); + + $styles = $out->makeResourceLoaderLink( 'jquery.qunit', ResourceLoaderModule::TYPE_STYLES, false ); + // Use 'raw' since this is a plain HTML page without ResourceLoader + $scripts = $out->makeResourceLoaderLink( 'jquery.qunit', ResourceLoaderModule::TYPE_SCRIPTS, false, array( 'raw' => 'true' ) ); + + $head = trim( $styles['html'] . $scripts['html'] ); + $html = <<<HTML +<!DOCTYPE html> +<title>QUnit</title> +$head +<div id="qunit"></div> +HTML; + $html .= "\n" . Html::linkedScript( $url ); + + header( 'Content-Type: text/html; charset=utf-8' ); + echo $html; } /** @@ -183,7 +279,7 @@ HTML; return self::prefixSearchArray( $search, $limit, - array_keys( self::$frameworks ) + self::$frameworks ); } diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 6de7c90d..24b636b1 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -538,14 +538,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; } } diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 89ce2b3a..14959c26 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1416,27 +1416,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" ); @@ -1528,7 +1523,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; } |