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

ChangeLog
t3lib/class.t3lib_tceforms.php
typo3/class.browse_links.php
typo3/wizard_colorpicker.php
typo3/wizard_tsconfig.php

index 85b2296..dc0fa43 100755 (executable)
--- a/ChangeLog
+++ b/ChangeLog
@@ -19,6 +19,7 @@
        * 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)
        * Follow-up to bug #14389: Added unit test
+       * Fixed bug #1985: XSS vulnerability in wizard classes
 
 2010-07-27  Steffen Kamper  <steffen@typo3.org>
 
index aae284e..6e62370 100644 (file)
@@ -3896,6 +3896,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 f3b39ed..9ae80bf 100644 (file)
@@ -922,6 +922,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']);
                        $update='';
                        foreach ($this->P['fieldChangeFunc'] as $k => $v) {
@@ -2792,6 +2795,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']))
+               );
+       }
 }
 
 
index 3de035b..891f5b1 100644 (file)
@@ -77,6 +77,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.
@@ -114,6 +115,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');
@@ -132,7 +134,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.= '
@@ -268,7 +270,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">
@@ -434,6 +437,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)
+               );
+       }
 }
 
 
index b4e7efa..e66e21c 100644 (file)
@@ -156,7 +156,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)  {
@@ -621,6 +623,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']))
+               );
+       }
 }