diff options
Diffstat (limited to 'includes')
| -rw-r--r-- | includes/DefaultSettings.php | 23 | ||||
| -rw-r--r-- | includes/EditPage.php | 18 | ||||
| -rw-r--r-- | includes/ImagePage.php | 2 | ||||
| -rw-r--r-- | includes/OutputHandler.php | 6 | ||||
| -rw-r--r-- | includes/OutputPage.php | 63 | ||||
| -rw-r--r-- | includes/Sanitizer.php | 51 | ||||
| -rw-r--r-- | includes/User.php | 1 | ||||
| -rw-r--r-- | includes/XmlTypeCheck.php | 57 | ||||
| -rw-r--r-- | includes/api/ApiBase.php | 1 | ||||
| -rw-r--r-- | includes/api/ApiEditPage.php | 3 | ||||
| -rw-r--r-- | includes/api/ApiFormatJson.php | 14 | ||||
| -rw-r--r-- | includes/api/ApiFormatPhp.php | 19 | ||||
| -rw-r--r-- | includes/api/ApiMain.php | 2 | ||||
| -rw-r--r-- | includes/api/ApiQueryLogEvents.php | 8 | ||||
| -rw-r--r-- | includes/db/DatabaseOracle.php | 4 | ||||
| -rw-r--r-- | includes/filerepo/file/LocalFile.php | 2 | ||||
| -rw-r--r-- | includes/installer/Installer.php | 7 | ||||
| -rw-r--r-- | includes/parser/ParserOutput.php | 13 | ||||
| -rw-r--r-- | includes/upload/UploadBase.php | 104 |
19 files changed, 347 insertions, 51 deletions
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 4eb979ac..78568107 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -63,7 +63,7 @@ $wgConf = new SiteConfiguration; * MediaWiki version number * @since 1.2 */ -$wgVersion = '1.22.8'; +$wgVersion = '1.22.15'; /** * Name of the site. It must be changed in LocalSettings.php @@ -3322,6 +3322,27 @@ $wgResourceLoaderLESSImportPaths = array( "$IP/resources/mediawiki.less/", ); +/** + * Whether to allow site-wide CSS (MediaWiki:Common.css and friends) on + * restricted pages like Special:UserLogin or Special:Preferences where + * JavaScript is disabled for security reasons. As it is possible to + * execute JavaScript through CSS, setting this to true opens up a + * potential security hole. Some sites may "skin" their wiki by using + * site-wide CSS, causing restricted pages to look unstyled and different + * from the rest of the site. + * + * @since 1.25 + */ +$wgAllowSiteCSSOnRestrictedPages = false; + +/** + * When OutputHandler is used, mangle any output that contains + * <cross-domain-policy>. Without this, an attacker can send their own + * cross-domain policy unless it is prevented by the crossdomain.xml file at + * the domain root. + */ +$wgMangleFlashPolicy = true; + /** @} */ # End of resource loader settings } /*************************************************************************//** diff --git a/includes/EditPage.php b/includes/EditPage.php index 16d9a5a4..4dd83845 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -156,6 +156,12 @@ class EditPage { const AS_IMAGE_REDIRECT_LOGGED = 234; /** + * Status: user tried to modify the content model, but is not allowed to do that + * ( User::isAllowed('editcontentmodel') == false ) + */ + const AS_NO_CHANGE_CONTENT_MODEL = 235; + + /** * Status: can't parse content */ const AS_PARSE_ERROR = 240; @@ -1289,6 +1295,9 @@ class EditPage { $permission = $this->mTitle->isTalkPage() ? 'createtalk' : 'createpage'; throw new PermissionsError( $permission ); + case self::AS_NO_CHANGE_CONTENT_MODEL: + throw new PermissionsError( 'editcontentmodel' ); + default: // We don't recognize $status->value. The only way that can happen // is if an extension hook aborted from inside ArticleSave. @@ -1503,6 +1512,15 @@ class EditPage { } } + if ( $this->contentModel !== $this->mTitle->getContentModel() + && !$wgUser->isAllowed( 'editcontentmodel' ) + ) { + $status->setResult( false, self::AS_NO_CHANGE_CONTENT_MODEL ); + wfProfileOut( __METHOD__ . '-checks' ); + wfProfileOut( __METHOD__ ); + return $status; + } + if ( wfReadOnly() ) { $status->fatal( 'readonlytext' ); $status->value = self::AS_READ_ONLY_PAGE; diff --git a/includes/ImagePage.php b/includes/ImagePage.php index 7ea06b0e..d696a17c 100644 --- a/includes/ImagePage.php +++ b/includes/ImagePage.php @@ -420,6 +420,8 @@ class ImagePage extends Article { if ( $page > 1 ) { $label = $out->parse( wfMessage( 'imgmultipageprev' )->text(), false ); + // on the client side, this link is generated in ajaxifyPageNavigation() + // in the mediawiki.page.image.pagination module $link = Linker::linkKnown( $this->getTitle(), $label, diff --git a/includes/OutputHandler.php b/includes/OutputHandler.php index 3860b8e2..65bb86e7 100644 --- a/includes/OutputHandler.php +++ b/includes/OutputHandler.php @@ -28,8 +28,10 @@ * @return string */ function wfOutputHandler( $s ) { - global $wgDisableOutputCompression, $wgValidateAllHtml; - $s = wfMangleFlashPolicy( $s ); + global $wgDisableOutputCompression, $wgValidateAllHtml, $wgMangleFlashPolicy; + if ( $wgMangleFlashPolicy ) { + $s = wfMangleFlashPolicy( $s ); + } if ( $wgValidateAllHtml ) { $headers = headers_list(); $isHTML = false; diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 7f0454f6..e6d4339f 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -151,9 +151,11 @@ class OutputPage extends ContextSource { var $mFeedLinksAppendQuery = null; - # What level of 'untrustworthiness' is allowed in CSS/JS modules loaded on this page? - # @see ResourceLoaderModule::$origin - # ResourceLoaderModule::ORIGIN_ALL is assumed unless overridden; + /** @var array + * What level of 'untrustworthiness' is allowed in CSS/JS modules loaded on this page? + * @see ResourceLoaderModule::$origin + * ResourceLoaderModule::ORIGIN_ALL is assumed unless overridden; + */ protected $mAllowedModules = array( ResourceLoaderModule::TYPE_COMBINED => ResourceLoaderModule::ORIGIN_ALL, ); @@ -1273,12 +1275,29 @@ class OutputPage extends ContextSource { /** * Do not allow scripts which can be modified by wiki users to load on this page; * only allow scripts bundled with, or generated by, the software. + * Site-wide styles are controlled by a config setting, since they can be + * used to create a custom skin/theme, but not user-specific ones. + * + * @todo this should be given a more accurate name */ public function disallowUserJs() { + global $wgAllowSiteCSSOnRestrictedPages; $this->reduceAllowedModules( ResourceLoaderModule::TYPE_SCRIPTS, ResourceLoaderModule::ORIGIN_CORE_INDIVIDUAL ); + + // Site-wide styles are controlled by a config setting, see bug 71621 + // for background on why. User styles are never allowed. + if ( $wgAllowSiteCSSOnRestrictedPages ) { + $styleOrigin = ResourceLoaderModule::ORIGIN_USER_SITEWIDE; + } else { + $styleOrigin = ResourceLoaderModule::ORIGIN_CORE_INDIVIDUAL; + } + $this->reduceAllowedModules( + ResourceLoaderModule::TYPE_STYLES, + $styleOrigin + ); } /** @@ -1293,10 +1312,11 @@ class OutputPage extends ContextSource { } /** - * Show what level of JavaScript / CSS untrustworthiness is allowed on this page + * Get the level of JavaScript / CSS untrustworthiness allowed on this page. + * * @see ResourceLoaderModule::$origin * @param string $type ResourceLoaderModule TYPE_ constant - * @return Int ResourceLoaderModule ORIGIN_ class constant + * @return int ResourceLoaderModule ORIGIN_ class constant */ public function getAllowedModules( $type ) { if ( $type == ResourceLoaderModule::TYPE_COMBINED ) { @@ -1310,17 +1330,26 @@ class OutputPage extends ContextSource { /** * Set the highest level of CSS/JS untrustworthiness allowed - * @param $type String ResourceLoaderModule TYPE_ constant - * @param $level Int ResourceLoaderModule class constant + * + * @deprecated since 1.24 Raising level of allowed untrusted content is no longer supported. + * Use reduceAllowedModules() instead + * + * @param string $type ResourceLoaderModule TYPE_ constant + * @param int $level ResourceLoaderModule class constant */ public function setAllowedModules( $type, $level ) { - $this->mAllowedModules[$type] = $level; + wfDeprecated( __METHOD__, '1.24' ); + $this->reduceAllowedModules( $type, $level ); } /** - * As for setAllowedModules(), but don't inadvertently make the page more accessible - * @param $type String - * @param $level Int ResourceLoaderModule class constant + * Limit the highest level of CSS/JS untrustworthiness allowed. + * + * If passed the same or a higher level than the current level of untrustworthiness set, the + * level will remain unchanged. + * + * @param string $type + * @param int $level ResourceLoaderModule class constant */ public function reduceAllowedModules( $type, $level ) { $this->mAllowedModules[$type] = min( $this->getAllowedModules( $type ), $level ); @@ -1574,6 +1603,8 @@ class OutputPage extends ContextSource { $this->addModuleScripts( $parserOutput->getModuleScripts() ); $this->addModuleStyles( $parserOutput->getModuleStyles() ); $this->addModuleMessages( $parserOutput->getModuleMessages() ); + $this->mPreventClickjacking = $this->mPreventClickjacking + || $parserOutput->preventClickjacking(); // Template versioning... foreach ( (array)$parserOutput->getTemplateIds() as $ns => $dbks ) { @@ -1874,6 +1905,16 @@ class OutputPage extends ContextSource { } /** + * Get the prevent-clickjacking flag + * + * @since 1.24 + * @return boolean + */ + public function getPreventClickjacking() { + return $this->mPreventClickjacking; + } + + /** * Get the X-Frame-Options header value (without the name part), or false * if there isn't one. This is used by Skin to determine whether to enable * JavaScript frame-breaking, for clients that don't support X-Frame-Options. diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php index 9e9ac38b..3ca66443 100644 --- a/includes/Sanitizer.php +++ b/includes/Sanitizer.php @@ -819,24 +819,16 @@ class Sanitizer { } /** - * Pick apart some CSS and check it for forbidden or unsafe structures. - * Returns a sanitized string. This sanitized string will have - * character references and escape sequences decoded and comments - * stripped (unless it is itself one valid comment, in which case the value - * will be passed through). If the input is just too evil, only a comment - * complaining about evilness will be returned. - * - * Currently URL references, 'expression', 'tps' are forbidden. - * - * NOTE: Despite the fact that character references are decoded, the - * returned string may contain character references given certain - * clever input strings. These character references must - * be escaped before the return value is embedded in HTML. - * - * @param $value String - * @return String + * Normalize CSS into a format we can easily search for hostile input + * - decode character references + * - decode escape sequences + * - convert characters that IE6 interprets into ascii + * - remove comments, unless the entire value is one single comment + * @param string $value the css string + * @return string normalized css */ - static function checkCss( $value ) { + public static function normalizeCss( $value ) { + // Decode character references like { $value = Sanitizer::decodeCharReferences( $value ); @@ -922,6 +914,31 @@ class Sanitizer { $value ); + return $value; + } + + + /** + * Pick apart some CSS and check it for forbidden or unsafe structures. + * Returns a sanitized string. This sanitized string will have + * character references and escape sequences decoded and comments + * stripped (unless it is itself one valid comment, in which case the value + * will be passed through). If the input is just too evil, only a comment + * complaining about evilness will be returned. + * + * Currently URL references, 'expression', 'tps' are forbidden. + * + * NOTE: Despite the fact that character references are decoded, the + * returned string may contain character references given certain + * clever input strings. These character references must + * be escaped before the return value is embedded in HTML. + * + * @param string $value + * @return string + */ + static function checkCss( $value ) { + $value = self::normalizeCss( $value ); + // Reject problematic keywords and control characters if ( preg_match( '/[\000-\010\013\016-\037\177]/', $value ) ) { return '/* invalid control char */'; diff --git a/includes/User.php b/includes/User.php index 62324043..4c7a39df 100644 --- a/includes/User.php +++ b/includes/User.php @@ -122,6 +122,7 @@ class User { 'deletelogentry', 'deleterevision', 'edit', + 'editcontentmodel', 'editinterface', 'editprotected', 'editmyoptions', diff --git a/includes/XmlTypeCheck.php b/includes/XmlTypeCheck.php index eb98a4ad..aca857e9 100644 --- a/includes/XmlTypeCheck.php +++ b/includes/XmlTypeCheck.php @@ -40,6 +40,23 @@ class XmlTypeCheck { public $rootElement = ''; /** + * A stack of strings containing the data of each xml element as it's processed. Append + * data to the top string of the stack, then pop off the string and process it when the + * element is closed. + */ + protected $elementData = array(); + + /** + * A stack of element names and attributes, as we process them. + */ + protected $elementDataContext = array(); + + /** + * Current depth of the data stack. + */ + protected $stackDepth = 0; + + /** * Additional parsing options */ private $parserOptions = array( @@ -51,7 +68,7 @@ class XmlTypeCheck { * @param callable $filterCallback (optional) * Function to call to do additional custom validity checks from the * SAX element handler event. This gives you access to the element - * namespace, name, and attributes, but not to text contents. + * namespace, name, attributes, and text contents. * Filter should return 'true' to toggle on $this->filterMatch * @param boolean $isFile (optional) indicates if the first parameter is a * filename (default, true) or if it is a string (false) @@ -179,7 +196,12 @@ class XmlTypeCheck { $this->rootElement = $name; if ( is_callable( $this->filterCallback ) ) { - xml_set_element_handler( $parser, array( $this, 'elementOpen' ), false ); + 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 @@ -193,7 +215,26 @@ class XmlTypeCheck { * @param $attribs */ private function elementOpen( $parser, $name, $attribs ) { - if ( call_user_func( $this->filterCallback, $name, $attribs ) ) { + $this->elementDataContext[] = array( $name, $attribs ); + $this->elementData[] = ''; + $this->stackDepth++; + } + + /** + * @param $parser + * @param $name + */ + private function elementClose( $parser, $name ) { + 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! $this->filterMatch = true; } @@ -201,6 +242,16 @@ class XmlTypeCheck { /** * @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 + $this->elementData[ $this->stackDepth - 1 ] .= trim( $data ); + } + + /** + * @param $parser * @param $target * @param $data */ diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index ce6ecda6..c1454e76 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -1351,6 +1351,7 @@ abstract class ApiBase extends ContextSource { 'permdenied-undelete' => array( 'code' => 'permissiondenied', 'info' => "You don't have permission to restore deleted revisions" ), 'createonly-exists' => array( 'code' => 'articleexists', 'info' => "The article you tried to create has been created already" ), 'nocreate-missing' => array( 'code' => 'missingtitle', 'info' => "The article you tried to edit doesn't exist" ), + 'cantchangecontentmodel' => array( 'code' => 'cantchangecontentmodel', 'info' => "You don't have permission to change the content model of a page" ), 'nosuchrcid' => array( 'code' => 'nosuchrcid', 'info' => "There is no change with rcid \"\$1\"" ), 'protect-invalidaction' => array( 'code' => 'protect-invalidaction', 'info' => "Invalid protection type \"\$1\"" ), 'protect-invalidlevel' => array( 'code' => 'protect-invalidlevel', 'info' => "Invalid protection level \"\$1\"" ), diff --git a/includes/api/ApiEditPage.php b/includes/api/ApiEditPage.php index bd61895b..51c9efc6 100644 --- a/includes/api/ApiEditPage.php +++ b/includes/api/ApiEditPage.php @@ -423,6 +423,9 @@ class ApiEditPage extends ApiBase { case EditPage::AS_NO_CREATE_PERMISSION: $this->dieUsageMsg( 'nocreate-loggedin' ); + case EditPage::AS_NO_CHANGE_CONTENT_MODEL: + $this->dieUsageMsg( 'cantchangecontentmodel' ); + case EditPage::AS_BLANK_ARTICLE: $this->dieUsageMsg( 'blankpage' ); diff --git a/includes/api/ApiFormatJson.php b/includes/api/ApiFormatJson.php index 342a580f..47d82124 100644 --- a/includes/api/ApiFormatJson.php +++ b/includes/api/ApiFormatJson.php @@ -62,10 +62,22 @@ class ApiFormatJson extends ApiFormatBase { $this->getIsHtml(), $params['utf8'] ? FormatJson::ALL_OK : FormatJson::XMLMETA_OK ); + + // Bug 66776: wfMangleFlashPolicy() is needed to avoid a nasty bug in + // Flash, but what it does isn't friendly for the API, so we need to + // work around it. + if ( preg_match( '/\<\s*cross-domain-policy\s*\>/i', $json ) ) { + $json = preg_replace( + '/\<(\s*cross-domain-policy\s*)\>/i', '\\u003C$1\\u003E', $json + ); + } + $callback = $params['callback']; if ( $callback !== null ) { $callback = preg_replace( "/[^][.\\'\\\"_A-Za-z0-9]/", '', $callback ); - $this->printText( "$callback($json)" ); + # Prepend a comment to try to avoid attacks against content + # sniffers, such as bug 68187. + $this->printText( "/**/$callback($json)" ); } else { $this->printText( $json ); } diff --git a/includes/api/ApiFormatPhp.php b/includes/api/ApiFormatPhp.php index b2d1f044..bda1c180 100644 --- a/includes/api/ApiFormatPhp.php +++ b/includes/api/ApiFormatPhp.php @@ -35,7 +35,24 @@ class ApiFormatPhp extends ApiFormatBase { } public function execute() { - $this->printText( serialize( $this->getResultData() ) ); + global $wgMangleFlashPolicy; + $text = serialize( $this->getResultData() ); + + // Bug 66776: wfMangleFlashPolicy() is needed to avoid a nasty bug in + // Flash, but what it does isn't friendly for the API. There's nothing + // we can do here that isn't actively broken in some manner, so let's + // just be broken in a useful manner. + if ( $wgMangleFlashPolicy && + in_array( 'wfOutputHandler', ob_list_handlers(), true ) && + preg_match( '/\<\s*cross-domain-policy\s*\>/i', $text ) + ) { + $this->dieUsage( + 'This response cannot be represented using format=php. See https://bugzilla.wikimedia.org/show_bug.cgi?id=66776', + 'internalerror' + ); + } + + $this->printText( $text ); } public function getDescription() { diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index c11f16cb..ea2fcc78 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -510,7 +510,7 @@ class ApiMain extends ApiBase { array( '.*?', '.' ), $wildcard ); - return "/https?:\/\/$wildcard/"; + return "/^https?:\/\/$wildcard$/"; } protected function sendCacheHeaders() { diff --git a/includes/api/ApiQueryLogEvents.php b/includes/api/ApiQueryLogEvents.php index 26774ef4..ecd117e4 100644 --- a/includes/api/ApiQueryLogEvents.php +++ b/includes/api/ApiQueryLogEvents.php @@ -36,7 +36,7 @@ class ApiQueryLogEvents extends ApiQueryBase { } private $fld_ids = false, $fld_title = false, $fld_type = false, - $fld_action = false, $fld_user = false, $fld_userid = false, + $fld_user = false, $fld_userid = false, $fld_timestamp = false, $fld_comment = false, $fld_parsedcomment = false, $fld_details = false, $fld_tags = false; @@ -49,7 +49,6 @@ class ApiQueryLogEvents extends ApiQueryBase { $this->fld_ids = isset( $prop['ids'] ); $this->fld_title = isset( $prop['title'] ); $this->fld_type = isset( $prop['type'] ); - $this->fld_action = isset( $prop['action'] ); $this->fld_user = isset( $prop['user'] ); $this->fld_userid = isset( $prop['userid'] ); $this->fld_timestamp = isset( $prop['timestamp'] ); @@ -304,6 +303,9 @@ class ApiQueryLogEvents extends ApiQueryBase { if ( LogEventsList::isDeleted( $row, LogPage::DELETED_ACTION ) ) { $vals['actionhidden'] = ''; } else { + if ( $this->fld_type ) { + $vals['action'] = $row->log_action; + } if ( $this->fld_title ) { ApiQueryBase::addTitleInfo( $vals, $title ); } @@ -313,7 +315,7 @@ class ApiQueryLogEvents extends ApiQueryBase { } } - if ( $this->fld_type || $this->fld_action ) { + if ( $this->fld_type ) { $vals['type'] = $row->log_type; $vals['action'] = $row->log_action; } diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index fb2d4359..9009b328 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -1304,6 +1304,10 @@ class DatabaseOracle extends DatabaseBase { throw new DBUnexpectedError( $this, "Cannot create LOB descriptor: " . $e['message'] ); } + if ( is_object( $val ) ) { + $val = $val->getData(); + } + if ( $col_type == 'BLOB' ) { $lob[$col]->writeTemporary( $val ); oci_bind_by_name( $stmt, ":$col", $lob[$col], - 1, SQLT_BLOB ); diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index fe769be2..d18f42e4 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -423,6 +423,8 @@ class LocalFile extends File { $decoded['timestamp'] = wfTimestamp( TS_MW, $decoded['timestamp'] ); + $decoded['metadata'] = $this->repo->getSlaveDB()->decodeBlob( $decoded['metadata'] ); + if ( empty( $decoded['major_mime'] ) ) { $decoded['mime'] = 'unknown/unknown'; } else { diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index f248d859..a9908134 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -515,6 +515,13 @@ abstract class Installer { public static function getExistingLocalSettings() { global $IP; + // You might be wondering why this is here. Well if you don't do this + // then some poorly-formed extensions try to call their own classes + // after immediately registering them. We really need to get extension + // registration out of the global scope and into a real format. + // @see https://bugzilla.wikimedia.org/67440 + global $wgAutoloadClasses; + wfSuppressWarnings(); $_lsExists = file_exists( "$IP/LocalSettings.php" ); wfRestoreWarnings(); diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php index 502f0fd1..460f3211 100644 --- a/includes/parser/ParserOutput.php +++ b/includes/parser/ParserOutput.php @@ -55,6 +55,7 @@ class ParserOutput extends CacheTime { private $mExtensionData = array(); # extra data used by extensions private $mLimitReportData = array(); # Parser limit report data private $mParseStartTime = array(); # Timestamps for getTimeSinceStart() + private $mPreventClickjacking = false; # Whether to emit X-Frame-Options: DENY const EDITSECTION_REGEX = '#<(?:mw:)?editsection page="(.*?)" section="(.*?)"(?:/>|>(.*?)(</(?:mw:)?editsection>))#'; @@ -330,6 +331,7 @@ class ParserOutput extends CacheTime { $this->addModuleMessages( $out->getModuleMessages() ); $this->mHeadItems = array_merge( $this->mHeadItems, $out->getHeadItemsArray() ); + $this->mPreventClickjacking = $this->mPreventClickjacking || $out->getPreventClickjacking(); } /** @@ -629,4 +631,15 @@ class ParserOutput extends CacheTime { function setLimitReportData( $key, $value ) { $this->mLimitReportData[$key] = $value; } + + /** + * Get or set the prevent-clickjacking flag + * + * @since 1.24 + * @param boolean|null $flag New flag value, or null to leave it unchanged + * @return boolean Old flag value + */ + public function preventClickjacking( $flag = null ) { + return wfSetVar( $this->mPreventClickjacking, $flag ); + } } diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 40b3b19a..8268f8e3 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1201,7 +1201,8 @@ abstract class UploadBase { * @param $attribs array * @return bool */ - public function checkSvgScriptCallback( $element, $attribs ) { + public function checkSvgScriptCallback( $element, $attribs, $data = null ) { + list( $namespace, $strippedElement ) = $this->splitXmlNamespace( $element ); static $validNamespaces = array( @@ -1274,6 +1275,14 @@ abstract class UploadBase { return true; } + # Check <style> css + if ( $strippedElement == 'style' + && self::checkCssFragment( Sanitizer::normalizeCss( $data ) ) + ) { + wfDebug( __METHOD__ . ": hostile css in style element.\n" ); + return true; + } + foreach ( $attribs as $attrib => $value ) { $stripped = $this->stripXmlNamespace( $attrib ); $value = strtolower( $value ); @@ -1294,8 +1303,8 @@ abstract class UploadBase { wfDebug( __METHOD__ . ": Found href attribute <$strippedElement " . "'$attrib'='$value' in uploaded file.\n" ); - return true; - } + return true; + } } # href with embedded svg as target @@ -1310,6 +1319,18 @@ abstract class UploadBase { 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' + && $strippedElement === 'animate' + && !preg_match( '!^https?://!im', $value ) + ) { + wfDebug( __METHOD__ . ": Found animate that might be changing href using from " + . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" ); + + return true; + } + # use set/animate to add event-handler attribute to parent if ( ( $strippedElement == 'set' || $strippedElement == 'animate' ) && $stripped == 'attributename' && substr( $value, 0, 2 ) == 'on' ) { wfDebug( __METHOD__ . ": Found svg setting event-handler attribute with \"<$strippedElement $stripped='$value'...\" in uploaded file.\n" ); @@ -1335,14 +1356,23 @@ abstract class UploadBase { } # use CSS styles to bring in remote code - # catch url("http:..., url('http:..., url(http:..., but not url("#..., url('#..., url(#.... - if ( $stripped == 'style' && preg_match_all( '!((?:font|clip-path|fill|filter|marker|marker-end|marker-mid|marker-start|mask|stroke)\s*:\s*url\s*\(\s*["\']?\s*[^#]+.*?\))!sim', $value, $matches ) ) { - foreach ( $matches[1] as $match ) { - if ( !preg_match( '!(?:font|clip-path|fill|filter|marker|marker-end|marker-mid|marker-start|mask|stroke)\s*:\s*url\s*\(\s*(#|\'#|"#)!sim', $match ) ) { - wfDebug( __METHOD__ . ": Found svg setting a style with remote url '$attrib'='$value' in uploaded file.\n" ); - return true; - } - } + if ( $stripped == 'style' + && self::checkCssFragment( Sanitizer::normalizeCss( $value ) ) + ) { + wfDebug( __METHOD__ . ": Found svg setting a style with " + . "remote url '$attrib'='$value' in uploaded file.\n" ); + return true; + } + + # Several attributes can include css, css character escaping isn't allowed + $cssAttrs = array( 'font', 'clip-path', 'fill', 'filter', 'marker', + 'marker-end', 'marker-mid', 'marker-start', 'mask', 'stroke' ); + if ( in_array( $stripped, $cssAttrs ) + && self::checkCssFragment( $value ) + ) { + wfDebug( __METHOD__ . ": Found svg setting a style with " + . "remote url '$attrib'='$value' in uploaded file.\n" ); + return true; } # image filters can pull in url, which could be svg that executes scripts @@ -1357,6 +1387,58 @@ abstract class UploadBase { } /** + * Check a block of CSS or CSS fragment for anything that looks like + * it is bringing in remote code. + * @param string $value a string of CSS + * @param bool $propOnly only check css properties (start regex with :) + * @return bool true if the CSS contains an illegal string, false if otherwise + */ + private static function checkCssFragment( $value ) { + + # Forbid external stylesheets, for both reliability and to protect viewer's privacy + if ( strpos( $value, '@import' ) !== false ) { + return true; + } + + # We allow @font-face to embed fonts with data: urls, so we snip the string + # 'url' out so this case won't match when we check for urls below + $pattern = '!(@font-face\s*{[^}]*src:)url(\("data:;base64,)!im'; + $value = preg_replace( $pattern, '$1$2', $value ); + + # Check for remote and executable CSS. Unlike in Sanitizer::checkCss, the CSS + # properties filter and accelerator don't seem to be useful for xss in SVG files. + # Expression and -o-link don't seem to work either, but filtering them here in case. + # Additionally, we catch remote urls like url("http:..., url('http:..., url(http:..., + # but not local ones such as url("#..., url('#..., url(#.... + if ( preg_match( '!expression + | -o-link\s*: + | -o-link-source\s*: + | -o-replace\s*:!imx', $value ) ) { + return true; + } + + if ( preg_match_all( + "!(\s*(url|image|image-set)\s*\(\s*[\"']?\s*[^#]+.*?\))!sim", + $value, + $matches + ) !== 0 + ) { + # TODO: redo this in one regex. Until then, url("#whatever") matches the first + foreach ( $matches[1] as $match ) { + if ( !preg_match( "!\s*(url|image|image-set)\s*\(\s*(#|'#|\"#)!im", $match ) ) { + return true; + } + } + } + + if ( preg_match( '/[\000-\010\013\016-\037\177]/', $value ) ) { + return true; + } + + return false; + } + + /** * Divide the element name passed by the xml parser to the callback into URI and prifix. * @param $name string * @return array containing the namespace URI and prefix |