SECURITY: XSS in langconverter when regex hits pcre.backtrack_limit

[lhc/web/wiklou.git] / languages / LanguageConverter.php

diff --git a/languages/LanguageConverter.php b/languages/LanguageConverter.php
index 4c3e5be..00bc02d 100644 (file)
--- a/languages/LanguageConverter.php
+++ b/languages/LanguageConverter.php
@@ -20,6 +20,8 @@
  */
 use MediaWiki\MediaWikiServices;
 
+use MediaWiki\Logger\LoggerFactory;
+
 /**
  * Base class for language conversion.
  * @ingroup Language
@@ -36,6 +38,7 @@ class LanguageConverter {
         * @var array
         */
        static public $languagesWithVariants = [
+               'en',
                'gan',
                'iu',
                'kk',
@@ -60,11 +63,6 @@ class LanguageConverter {
        // 'bidirectional' 'unidirectional' 'disable' for each variant
        public $mManualLevel;
 
-       /**
-        * @var string Memcached key name
-        */
-       public $mCacheKey;
-
        public $mLangObj;
        public $mFlags;
        public $mDescCodeSep = ':', $mDescVarSep = ';';
@@ -95,7 +93,6 @@ class LanguageConverter {
                $this->mVariants = array_diff( $variants, $wgDisabledVariants );
                $this->mVariantFallbacks = $variantfallbacks;
                $this->mVariantNames = Language::fetchLanguageNames();
-               $this->mCacheKey = wfMemcKey( 'conversiontables', $maincode );
                $defaultflags = [
                        // 'S' show converted text
                        // '+' add rules for alltext
@@ -344,7 +341,6 @@ class LanguageConverter {
         * @return string The converted text
         */
        public function autoConvert( $text, $toVariant = false ) {
-
                $this->loadTables();
 
                if ( !$toVariant ) {
@@ -359,24 +355,30 @@ class LanguageConverter {
                }
 
                /* we convert everything except:
-                * 1. HTML markups (anything between < and >)
-                * 2. HTML entities
-                * 3. placeholders created by the parser
-                */
-               $marker = '|' . Parser::MARKER_PREFIX . '[\-a-zA-Z0-9]+';
+                  1. HTML markups (anything between < and >)
+                  2. HTML entities
+                  3. placeholders created by the parser
+                  IMPORTANT: Beware of failure from pcre.backtrack_limit (T124404).
+                  Minimize use of backtracking where possible.
+               */
+               $marker = '|' . Parser::MARKER_PREFIX . '[^\x7f]++\x7f';
 
                // this one is needed when the text is inside an HTML markup
-               $htmlfix = '|<[^>]+$|^[^<>]*>';
+               $htmlfix = '|<[^>\004]++(?=\004$)|^[^<>]*+>';
+
+               // Optimize for the common case where these tags have
+               // few or no children. Thus try and possesively get as much as
+               // possible, and only engage in backtracking when we hit a '<'.
 
                // disable convert to variants between <code> tags
-               $codefix = '<code>.+?<\/code>|';
+               $codefix = '<code>[^<]*+(?:(?:(?!<\/code>).)[^<]*+)*+<\/code>|';
                // disable conversion of <script> tags
-               $scriptfix = '<script.*?>.*?<\/script>|';
+               $scriptfix = '<script[^>]*+>[^<]*+(?:(?:(?!<\/script>).)[^<]*+)*+<\/script>|';
                // disable conversion of <pre> tags
-               $prefix = '<pre.*?>.*?<\/pre>|';
+               $prefix = '<pre[^>]*+>[^<]*+(?:(?:(?!<\/pre>).)[^<]*+)*+<\/pre>|';
 
                $reg = '/' . $codefix . $scriptfix . $prefix .
-                       '<[^>]+>|&[a-zA-Z#][a-z0-9]+;' . $marker . $htmlfix . '/s';
+                       '<[^>]++>|&[a-zA-Z#][a-z0-9]++;' . $marker . $htmlfix . '|\004$/s';
                $startPos = 0;
                $sourceBlob = '';
                $literalBlob = '';
@@ -387,15 +389,34 @@ class LanguageConverter {
 
                $markupMatches = null;
                $elementMatches = null;
+
+               // We add a marker (\004) at the end of text, to ensure we always match the
+               // entire text (Otherwise, pcre.backtrack_limit might cause silent failure)
                while ( $startPos < strlen( $text ) ) {
-                       if ( preg_match( $reg, $text, $markupMatches, PREG_OFFSET_CAPTURE, $startPos ) ) {
+                       if ( preg_match( $reg, $text . "\004", $markupMatches, PREG_OFFSET_CAPTURE, $startPos ) ) {
                                $elementPos = $markupMatches[0][1];
                                $element = $markupMatches[0][0];
+                               if ( $element === "\004" ) {
+                                       // We hit the end.
+                                       $elementPos = strlen( $text );
+                                       $element = '';
+                               }
                        } else {
-                               $elementPos = strlen( $text );
-                               $element = '';
+                               // If we hit here, then Language Converter could be tricked
+                               // into doing an XSS, so we refuse to translate.
+                               // If non-crazy input manages to reach this code path,
+                               // we should consider it a bug.
+                               $log = LoggerFactory::getInstance( 'languageconverter' );
+                               $log->error( "Hit pcre.backtrack_limit in " . __METHOD__
+                                       . ". Disabling language conversion for this page.",
+                                       array(
+                                               "method" => __METHOD__,
+                                               "variant" => $toVariant,
+                                               "startOfText" => substr( $text, 0, 500 )
+                                       )
+                               );
+                               return $text;
                        }
-
                        // Queue the part before the markup for translation in a batch
                        $sourceBlob .= substr( $text, $startPos, $elementPos - $startPos ) . "\000";
 
@@ -404,7 +425,7 @@ class LanguageConverter {
 
                        // Translate any alt or title attributes inside the matched element
                        if ( $element !== ''
-                               && preg_match( '/^(<[^>\s]*)\s([^>]*)(.*)$/', $element, $elementMatches )
+                               && preg_match( '/^(<[^>\s]*+)\s([^>]*+)(.*+)$/', $element, $elementMatches )
                        ) {
                                $attrs = Sanitizer::decodeTagAttributes( $elementMatches[2] );
                                $changed = false;
@@ -668,7 +689,7 @@ class LanguageConverter {
         *
         * @param string $text Text to be converted
         * @param string $variant The target variant code
-        * @param int $startPos
+        * @param int &$startPos
         * @param int $depth Depth of recursion
         *
         * @throws MWException
@@ -866,8 +887,9 @@ class LanguageConverter {
                $this->mTablesLoaded = true;
                $this->mTables = false;
                $cache = ObjectCache::getInstance( $wgLanguageConverterCacheType );
+               $cacheKey = $cache->makeKey( 'conversiontables', $this->mMainLanguageCode );
                if ( $fromCache ) {
-                       $this->mTables = $cache->get( $this->mCacheKey );
+                       $this->mTables = $cache->get( $cacheKey );
                }
                if ( !$this->mTables || !array_key_exists( self::CACHE_VERSION_KEY, $this->mTables ) ) {
                        // not in cache, or we need a fresh reload.
@@ -882,7 +904,7 @@ class LanguageConverter {
                        $this->postLoadTables();
                        $this->mTables[self::CACHE_VERSION_KEY] = true;
 
-                       $cache->set( $this->mCacheKey, $this->mTables, 43200 );
+                       $cache->set( $cacheKey, $this->mTables, 43200 );
                }
        }
 
@@ -895,9 +917,11 @@ class LanguageConverter {
        /**
         * Reload the conversion tables.
         *
+        * Also used by test suites which need to reset the converter state.
+        *
         * @private
         */
-       function reloadTables() {
+       private function reloadTables() {
                if ( $this->mTables ) {
                        unset( $this->mTables );
                }