[SECURITY] feuser_adminLib.inc allows to set arbitrary fields 78/26178/2
authorSteffen Ritter <info@rs-websystems.de>
Tue, 10 Dec 2013 09:50:53 +0000 (10:50 +0100)
committerOliver Hader <oliver.hader@typo3.org>
Tue, 10 Dec 2013 09:50:57 +0000 (10:50 +0100)
The CMS core ships a utility class helping extension authors
to create frontend-extension which need a mail-based opt-in.
This class is neither used by core nor really maintained.

In the opt-in process the fields which should be updated to
activate the user are put as URL parameter into the
activation link. In the default configuration this feature
set allows to set any values of any field to this record.

As a result a user could manipulate his activation link and
therefore extend his usergroups.

This patch ensures that all fields which are about to update
are added to the hash as well as only taking the values
from TypoScript so even if the fields match no harm can be
done.

Change-Id: Ie27fba37522f7f46894a962fbd9425c328ce0583
Fixes: #48187
Releases: 6.0, 4.7, 4.5
Security-Commit: 2c930f8f2a8d18b83bb9d2d49cbdbec839b47188
Security-Bulletin: TYPO3-CORE-SA-2013-004
Reviewed-on: https://review.typo3.org/26178
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
typo3/sysext/cms/tslib/media/scripts/fe_adminLib.inc
typo3/sysext/statictemplates/media/scripts/fe_adminLib.inc

index 0a31da9..8ad415b 100755 (executable)
@@ -1085,13 +1085,20 @@ class user_feAdmin      {
                        $fD = t3lib_div::_GP('fD');
                        $sFK = t3lib_div::_GP('sFK');
 
+                       $valuesConfiguredInTypoScript = isset($this->conf['setfixed.'][$sFK . '.']) ? $this->conf['setfixed.'][$sFK . '.'] : array();
+                       $fields = $valuesConfiguredInTypoScript;
+                       unset($fields['_FIELDLIST']);
+                       $fields = array_keys($fields);
+                       if (isset($valuesConfiguredInTypoScript['_FIELDLIST'])) {
+                               $fields = array_merge($fields, t3lib_div::trimExplode(',', $valuesConfiguredInTypoScript['_FIELDLIST']));
+                       }
+                       $valuesConfiguredInTypoScript['_FIELDLIST'] = implode(',', array_unique($fields));
+
                        $fieldArr=array();
-                       if (is_array($fD) || $sFK=='DELETE')    {
-                               if (is_array($fD))      {
-                                       foreach ($fD as $field => $value) {
-                                               $origArr[$field]=$value;
-                                               $fieldArr[]=$field;
-                                       }
+                       if (!empty($valuesConfiguredInTypoScript) || $sFK=='DELETE')    {
+                               foreach ($valuesConfiguredInTypoScript as $field => $value) {
+                                       $origArr[$field]=$value;
+                                       $fieldArr[]=$field;
                                }
                                $theCode = $this->setfixedHash($origArr,$origArr['_FIELDLIST']);
                                if (!strcmp($this->authCode,$theCode))  {
@@ -1099,7 +1106,8 @@ class user_feAdmin        {
                                                $this->cObj->DBgetDelete($this->theTable, $theUid, TRUE);
                                        } else {
                                                $newFieldList = implode(',',array_intersect(t3lib_div::trimExplode(',',$this->fieldList),t3lib_div::trimExplode(',',implode($fieldArr,','),1)));
-                                               $this->cObj->DBgetUpdate($this->theTable, $theUid, $fD, $newFieldList, TRUE);
+                                               unset($valuesConfiguredInTypoScript['_FIELDLIST']);
+                                               $this->cObj->DBgetUpdate($this->theTable, $theUid, $valuesConfiguredInTypoScript, $newFieldList, TRUE);
                                                $this->currentArr = $GLOBALS['TSFE']->sys_page->getRawRecord($this->theTable,$theUid);
                                                $this->userProcess_alt($this->conf['setfixed.']['userFunc_afterSave'],$this->conf['setfixed.']['userFunc_afterSave.'],array('rec'=>$this->currentArr, 'origRec'=>$origArr));
                                        }
@@ -1617,6 +1625,13 @@ class user_feAdmin       {
                                        $markerArray['###SYS_SETFIXED_DELETE###'] = $string;
                                        $markerArray['###SYS_SETFIXED_HSC_DELETE###'] = htmlspecialchars($string);
                                } elseif (strstr($theKey,'.'))  {
+                                       $fields = $data;
+                                       unset($fields['_FIELDLIST']);
+                                       $fields = array_keys($fields);
+                                       if (isset($data['_FIELDLIST'])) {
+                                               $fields = array_merge($fields, t3lib_div::trimExplode(',', $data['_FIELDLIST']));
+                                       }
+                                       $data['_FIELDLIST'] = implode(',', array_unique($fields));
                                        $theKey = substr($theKey,0,-1);
                                        if (is_array($data))    {
                                                $recCopy = $r;
index 82740d9..40a7543 100644 (file)
@@ -986,7 +986,7 @@ class user_feAdmin  {
                        if (!is_array($this->dataArr)) {
                                $this->dataArr = array();
                        }
-                       
+
                        $markerArray = $this->cObj->fillInMarkerArray($this->markerArray, $this->dataArr, '', TRUE, 'FIELD_', $this->recInMarkersHSC);
                        if ($this->conf['create.']['preview'] && !$this->previewLabel)  {$markerArray['###HIDDENFIELDS###'].= '<input type="hidden" name="preview" value="1" />';}
                        $content = $this->cObj->substituteMarkerArray($templateCode, $markerArray);
@@ -1087,13 +1087,20 @@ class user_feAdmin      {
                        $fD = t3lib_div::_GP('fD');
                        $sFK = t3lib_div::_GP('sFK');
 
+                       $valuesConfiguredInTypoScript = isset($this->conf['setfixed.'][$sFK . '.']) ? $this->conf['setfixed.'][$sFK . '.'] : array();
+                       $fields = $valuesConfiguredInTypoScript;
+                       unset($fields['_FIELDLIST']);
+                       $fields = array_keys($fields);
+                       if (isset($valuesConfiguredInTypoScript['_FIELDLIST'])) {
+                               $fields = array_merge($fields, t3lib_div::trimExplode(',', $valuesConfiguredInTypoScript['_FIELDLIST']));
+                       }
+                       $valuesConfiguredInTypoScript['_FIELDLIST'] = implode(',', array_unique($fields));
+
                        $fieldArr=array();
-                       if (is_array($fD) || $sFK=='DELETE')    {
-                               if (is_array($fD))      {
-                                       foreach ($fD as $field => $value) {
-                                               $origArr[$field]=$value;
-                                               $fieldArr[]=$field;
-                                       }
+                       if (!empty($valuesConfiguredInTypoScript) || $sFK=='DELETE')    {
+                               foreach ($valuesConfiguredInTypoScript as $field => $value) {
+                                       $origArr[$field]=$value;
+                                       $fieldArr[]=$field;
                                }
                                $theCode = $this->setfixedHash($origArr,$origArr['_FIELDLIST']);
                                if (!strcmp($this->authCode,$theCode))  {
@@ -1101,7 +1108,8 @@ class user_feAdmin        {
                                                $this->cObj->DBgetDelete($this->theTable, $theUid, TRUE);
                                        } else {
                                                $newFieldList = implode(',',array_intersect(t3lib_div::trimExplode(',',$this->fieldList),t3lib_div::trimExplode(',',implode($fieldArr,','),1)));
-                                               $this->cObj->DBgetUpdate($this->theTable, $theUid, $fD, $newFieldList, TRUE);
+                                               unset($valuesConfiguredInTypoScript['_FIELDLIST']);
+                                               $this->cObj->DBgetUpdate($this->theTable, $theUid, $valuesConfiguredInTypoScript, $newFieldList, TRUE);
                                                $this->currentArr = $GLOBALS['TSFE']->sys_page->getRawRecord($this->theTable,$theUid);
                                                $this->userProcess_alt($this->conf['setfixed.']['userFunc_afterSave'],$this->conf['setfixed.']['userFunc_afterSave.'],array('rec'=>$this->currentArr, 'origRec'=>$origArr));
                                        }
@@ -1619,6 +1627,13 @@ class user_feAdmin       {
                                        $markerArray['###SYS_SETFIXED_DELETE###'] = $string;
                                        $markerArray['###SYS_SETFIXED_HSC_DELETE###'] = htmlspecialchars($string);
                                } elseif (strstr($theKey,'.'))  {
+                                       $fields = $data;
+                                       unset($fields['_FIELDLIST']);
+                                       $fields = array_keys($fields);
+                                       if (isset($data['_FIELDLIST'])) {
+                                               $fields = array_merge($fields, t3lib_div::trimExplode(',', $data['_FIELDLIST']));
+                                       }
+                                       $data['_FIELDLIST'] = implode(',', array_unique($fields));
                                        $theKey = substr($theKey,0,-1);
                                        if (is_array($data))    {
                                                $recCopy = $r;