Fixed bug #1985: XSS vulnerability in wizard classes
authorOliver Hader <oliver.hader@typo3.org>
Wed, 28 Jul 2010 09:12:21 +0000 (09:12 +0000)
committerOliver Hader <oliver.hader@typo3.org>
Wed, 28 Jul 2010 09:12:21 +0000 (09:12 +0000)
git-svn-id: https://svn.typo3.org/TYPO3v4/Core/branches/TYPO3_4-2@8400 709f56b5-9817-0410-a4d7-c38de5d9e867

ChangeLog
t3lib/class.t3lib_div.php
t3lib/class.t3lib_tceforms.php
typo3/class.browse_links.php
typo3/sysext/rtehtmlarea/mod3/class.tx_rtehtmlarea_browse_links.php
typo3/wizard_colorpicker.php
typo3/wizard_tsconfig.php

index 0705db5..28f89f2 100755 (executable)
--- a/ChangeLog
+++ b/ChangeLog
@@ -18,6 +18,7 @@
        * Fixed bug #13885: XSS in indexed search BE module (thanks to Benjamin Mack)
        * Fixed bug #15254: Extension Manager allows to edit arbitrary files if noEdit flag is not set (thanks to Helmut Hummel)
        * Fixed bug #14389: phtml is also PHP extension and should be denied editing / uploading via fileadmin (thanks to Ernesto Baschny)
+       * Fixed bug #1985: XSS vulnerability in wizard classes
 
 2010-07-21  Ingo Renner  <ingo@typo3.org>
 
index a0458ef..c1ae580 100755 (executable)
@@ -1057,6 +1057,36 @@ final class t3lib_div {
        }
 
        /**
+        * Returns a proper HMAC on a given input string and secret TYPO3 encryption key.
+        *
+        * @param       string          Input string to create HMAC from
+        * @return      string          resulting (hexadecimal) HMAC currently with a length of 40 (HMAC-SHA-1)
+        */
+       public static function hmac($input) {
+               $hashAlgorithm = 'sha1';
+               $hashBlocksize = 64;
+               $hmac = '';
+
+               if (extension_loaded('hash') && function_exists('hash_hmac') && function_exists('hash_algos') && in_array($hashAlgorithm, hash_algos())) {
+                       $hmac = hash_hmac($hashAlgorithm, $input, $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']);
+               } else {
+                               // outer padding
+                       $opad = str_repeat(chr(0x5C), $hashBlocksize);
+                               // innner padding
+                       $ipad = str_repeat(chr(0x36), $hashBlocksize);
+                       if (strlen($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey']) > $hashBlocksize) {
+                                       // keys longer than blocksize are shorten
+                               $key = str_pad(pack('H*', call_user_func($hashAlgorithm, $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'])), $hashBlocksize, chr(0x00));
+                       } else {
+                                       // keys shorter than blocksize are zero-padded
+                               $key = str_pad($GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'], $hashBlocksize, chr(0x00));
+                       }
+                       $hmac = call_user_func($hashAlgorithm, ($key^$opad) . pack('H*', call_user_func($hashAlgorithm, ($key^$ipad) . $input)));
+               }
+               return $hmac;
+       }
+
+       /**
         * Takes comma-separated lists and arrays and removes all duplicates
         * If a value in the list is trim(empty), the value is ignored.
         * Usage: 16
index da4c166..0f001d7 100755 (executable)
@@ -3689,6 +3689,7 @@ class t3lib_TCEforms      {
                                                                        $params['formName'] = $this->formName;
                                                                        $params['itemName'] = $itemName;
                                                                        $params['fieldChangeFunc'] = $fieldChangeFunc;
+                                                                       $params['fieldChangeFuncHash'] = t3lib_div::hmac(serialize($fieldChangeFunc));
 
                                                                        switch((string)$wConf['type'])  {
                                                                                case 'popup':
index 111d4e5..904104b 100755 (executable)
@@ -932,6 +932,9 @@ class browse_links {
                ';
 
                if ($this->mode == 'wizard')    {       // Functions used, if the link selector is in wizard mode (= TCEforms fields)
+                       if (!$this->areFieldChangeFunctionsValid()) {
+                               $this->P['fieldChangeFunc'] = array();
+                       }
                        unset($this->P['fieldChangeFunc']['alert']);
                        reset($this->P['fieldChangeFunc']);
                        $update='';
@@ -2765,6 +2768,19 @@ class browse_links {
                }
                return $out;
        }
+
+       /**
+        * Determines whether submitted field change functions are valid
+        * and are coming from the system and not from an external abuse.
+        *
+        * @return boolean Whether the submitted field change functions are valid
+        */
+       protected function areFieldChangeFunctionsValid() {
+               return (
+                       isset($this->P['fieldChangeFunc']) && is_array($this->P['fieldChangeFunc']) && isset($this->P['fieldChangeFuncHash'])
+                       && $this->P['fieldChangeFuncHash'] == t3lib_div::hmac(serialize($this->P['fieldChangeFunc']))
+               );
+       }
 }
 
 // Include extension?
index 5ae1c75..32ba53d 100644 (file)
@@ -499,6 +499,9 @@ class tx_rtehtmlarea_browse_links extends browse_links {
 ';
 
                if ($this->mode=='wizard')      {       // Functions used, if the link selector is in wizard mode (= TCEforms fields)
+                       if (!$this->areFieldChangeFunctionsValid()) {
+                               $this->P['fieldChangeFunc'] = array();
+                       }
                        unset($this->P['fieldChangeFunc']['alert']);
                        reset($this->P['fieldChangeFunc']);
                        $update='';
index bceca98..0b4d262 100755 (executable)
@@ -78,6 +78,7 @@ class SC_wizard_colorpicker {
        var $P;                         // Wizard parameters, coming from TCEforms linking to the wizard.
        var $colorValue;        // Value of the current color picked.
        var $fieldChangeFunc;   // Serialized functions for changing the field... Necessary to call when the value is transferred to the TCEform since the form might need to do internal processing. Otherwise the value is simply not be saved.
+       protected $fieldChangeFuncHash;
        var $fieldName;         // Form name (from opener script)
        var $formName;          // Field name (from opener script)
        var $md5ID;                     // ID of element in opener script for which to set color.
@@ -115,6 +116,7 @@ class SC_wizard_colorpicker {
                        // Setting GET vars (used in colorpicker script):
                $this->colorValue = t3lib_div::_GP('colorValue');
                $this->fieldChangeFunc = t3lib_div::_GP('fieldChangeFunc');
+               $this->fieldChangeFuncHash = t3lib_div::_GP('fieldChangeFuncHash');
                $this->fieldName = t3lib_div::_GP('fieldName');
                $this->formName = t3lib_div::_GP('formName');
                $this->md5ID = t3lib_div::_GP('md5ID');
@@ -133,7 +135,7 @@ class SC_wizard_colorpicker {
                        // Setting field-change functions:
                $fieldChangeFuncArr = unserialize($this->fieldChangeFunc);
                $update = '';
-               if (is_array($fieldChangeFuncArr))      {
+               if ($this->areFieldChangeFunctionsValid()) {
                        unset($fieldChangeFuncArr['alert']);
                        foreach($fieldChangeFuncArr as $v)      {
                                $update.= '
@@ -270,7 +272,8 @@ class SC_wizard_colorpicker {
                                '&formName='.rawurlencode($this->P['formName']).
                                '&exampleImg='.rawurlencode($this->P['exampleImg']).
                                '&md5ID='.rawurlencode($this->P['md5ID']).
-                               '&fieldChangeFunc='.rawurlencode(serialize($this->P['fieldChangeFunc']));
+                               '&fieldChangeFunc='.rawurlencode(serialize($this->P['fieldChangeFunc'])) .
+                               '&fieldChangeFuncHash=' . $this->P['fieldChangeFuncHash'];
 
                $this->content.='
                        <frameset rows="*,1" framespacing="0" frameborder="0" border="0">
@@ -436,6 +439,19 @@ class SC_wizard_colorpicker {
                $hex = implode('',$hexvalue);
                return $hex;
        }
+
+       /**
+        * Determines whether submitted field change functions are valid
+        * and are coming from the system and not from an external abuse.
+        *
+        * @return boolean Whether the submitted field change functions are valid
+        */
+       protected function areFieldChangeFunctionsValid() {
+               return (
+                       $this->fieldChangeFunc && $this->fieldChangeFuncHash
+                       && $this->fieldChangeFuncHash == t3lib_div::hmac($this->fieldChangeFunc)
+               );
+       }
 }
 
 // Include extension?
index 97cc0db..3b5d556 100755 (executable)
@@ -164,7 +164,9 @@ class SC_wizard_tsconfig {
                $this->objString = t3lib_div::_GP('objString');
                $this->onlyProperty = t3lib_div::_GP('onlyProperty');
                        // Preparing some JavaScript code:
-               if (!is_array($this->P['fieldChangeFunc']))     $this->P['fieldChangeFunc']=array();
+               if (!$this->areFieldChangeFunctionsValid()) {
+                       $this->P['fieldChangeFunc']=array();
+               }
                unset($this->P['fieldChangeFunc']['alert']);
                $update='';
                foreach($this->P['fieldChangeFunc'] as $k=>$v)  {
@@ -637,6 +639,19 @@ class SC_wizard_tsconfig {
                        // Return link:
                return $out;
        }
+
+       /**
+        * Determines whether submitted field change functions are valid
+        * and are coming from the system and not from an external abuse.
+        *
+        * @return boolean Whether the submitted field change functions are valid
+        */
+       protected function areFieldChangeFunctionsValid() {
+               return (
+                       isset($this->P['fieldChangeFunc']) && is_array($this->P['fieldChangeFunc']) && isset($this->P['fieldChangeFuncHash'])
+                       && $this->P['fieldChangeFuncHash'] == t3lib_div::hmac(serialize($this->P['fieldChangeFunc']))
+               );
+       }
 }
 
 // Include extension?