[SECURITY] Fix SQL injection and XSS in record history
authorOliver Hader <oliver@typo3.org>
Thu, 8 Nov 2012 11:44:02 +0000 (12:44 +0100)
committerOliver Hader <oliver.hader@typo3.org>
Thu, 8 Nov 2012 11:44:05 +0000 (12:44 +0100)
This patch fixes the SQL injection possibilities in the record
history view as well as fixing XSS possibilities. The submitted
GET/POST data gets sanitized now besides that.

Change-Id: I033b296da0849736c989cfc1bb92e43546164b9c
Fixes: #42696
Releases: 6.0, 4.7, 4.6, 4.5
Security-Commit: 2bcfe757e38f326f3e7d8a52428f94f3945f9aa9
Security-Bulletin: TYPO3-CORE-SA-2012-005
Reviewed-on: http://review.typo3.org/16298
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
typo3/class.show_rechis.inc

index 33f17b7..6a4d9ef 100644 (file)
@@ -58,16 +58,26 @@ class recordHistory {
        var $changeLog;
        var $showMarked=FALSE;
        /**
+        * @var array
+        */
+       protected $recordCache = array();
+
+       /**
+        * @var array
+        */
+       protected $pageAccessCache = array();
+
+       /**
         * Constructor for the class
         *
         * @return      void
         */
        function recordHistory()        {
                        // GPvars:
-               $this->element = t3lib_div::_GP('element');
-               $this->returnUrl = t3lib_div::sanitizeLocalUrl(t3lib_div::_GP('returnUrl'));
-               $this->lastSyslogId = t3lib_div::_GP('diff');
-               $this->rollbackFields = t3lib_div::_GP('rollbackFields');
+               $this->element = $this->getArgument('element');
+               $this->returnUrl = $this->getArgument('returnUrl');
+               $this->lastSyslogId = $this->getArgument('diff');
+               $this->rollbackFields = $this->getArgument('rollbackFields');
                        // resolve sh_uid if set
                $this->resolveShUid();
        }
@@ -82,8 +92,8 @@ class recordHistory {
                $content = '';
 
                        // single-click rollback
-               if (t3lib_div::_GP('revert') && t3lib_div::_GP('sumUp'))        {
-                       $this->rollbackFields = t3lib_div::_GP('revert');
+               if ($this->getArgument('revert') && $this->getArgument('sumUp')) {
+                       $this->rollbackFields = $this->getArgument('revert');
                        $this->showInsertDelete = 0;
                        $this->showSubElements = 0;
 
@@ -99,8 +109,8 @@ class recordHistory {
                }
 
                        // save snapshot
-               if (t3lib_div::_GP('highlight') && !t3lib_div::_GP('settings')) {
-                       $this->toggleHighlight(t3lib_div::_GP('highlight'));
+               if ($this->getArgument('highlight') && !$this->getArgument('settings')) {
+                       $this->toggleHighlight($this->getArgument('highlight'));
                }
 
                $content .= $this->displaySettings();
@@ -271,7 +281,7 @@ class recordHistory {
                        // get current selection from UC, merge data, write it back to UC
                $currentSelection = is_array($BE_USER->uc['moduleData']['history']) ? $BE_USER->uc['moduleData']['history'] : array('maxSteps' => '', 'showDiff' => 1, 'showSubElements' => 1, 'showInsertDelete' => 1);
 
-               $currentSelectionOverride = t3lib_div::_GP('settings');
+               $currentSelectionOverride = $this->getArgument('settings');
                if ($currentSelectionOverride)  {
                        $currentSelection = array_merge($currentSelection,$currentSelectionOverride);
                        $BE_USER->uc['moduleData']['history'] = $currentSelection;
@@ -323,12 +333,15 @@ class recordHistory {
                $content = '';
                        // get link to page history if the element history is shown
                $elParts = explode(':',$this->element);
-               if ($elParts[0] != 'pages')     {
+               if (!empty($this->element) && $elParts[0] != 'pages') {
                        $content .= '<strong>'.$LANG->getLL('elementHistory',1).'</strong><br />';
-                       $pid = t3lib_BEfunc::getRecordRaw($elParts[0],'uid='.intval($elParts[1]));
-                       $content .= $this->linkPage($LANG->getLL('elementHistory_link',1),array('element' => 'pages:'.$pid['pid']));
+                       $pid = $this->getRecord($elParts[0], $elParts[1]);
+
+                       if ($this->hasPageAccess('pages', $pid['pid'])) {
+                               $content .= $this->linkPage($GLOBALS['LANG']->getLL('elementHistory_link', 1), array('element' => 'pages:' . $pid['pid']));
+                       }
                }
-               $content .= '<form name="settings" action="'.t3lib_div::getIndpEnv('TYPO3_REQUEST_URL').'" method="post"><table>'.$displayCode.'</table></form>';
+               $content .= '<form name="settings" action="'.htmlspecialchars(t3lib_div::getIndpEnv('TYPO3_REQUEST_URL')).'" method="post"><table>'.$displayCode.'</table></form>';
                return $SOBE->doc->section($LANG->getLL('settings',1),$content,0,1,0,0);
 
        }
@@ -647,10 +660,14 @@ class recordHistory {
 
                global $TCA;
                $elParts = explode(':',$this->element);
-               $changeLog = $this->getHistoryData($elParts[0],$elParts[1]);
 
+               if (empty($this->element)) {
+                       return 0;
+               }
+
+               $changeLog = $this->getHistoryData($elParts[0], $elParts[1]);
                        // get history of tables of this page and merge it into changelog
-               if ($elParts[0] == 'pages' && $this->showSubElements)   {
+               if ($elParts[0] == 'pages' && $this->showSubElements && $this->hasPageAccess('pages', $elParts[1])) {
                        foreach ($TCA as $tablename => $value)  {
                                $res = $GLOBALS['TYPO3_DB']->exec_SELECTquery('uid',$tablename,'pid='.intval($elParts[1]));     // check if there are records on the page
                                while ($row = $GLOBALS['TYPO3_DB']->sql_fetch_assoc($res))      {
@@ -681,9 +698,9 @@ class recordHistory {
         */
        function getHistoryData($table,$uid)    {
                global $TCA;
-               $uid = $this->resolveElement($table,$uid);
                        // If table is found in $TCA:
-               if ($TCA[$table])       {
+               if ($TCA[$table] && $this->hasTableAccess($table) && $this->hasPageAccess($table, $uid)) {
+                       $uid = $this->resolveElement($table, $uid);
                                // Selecting the $this->maxSteps most recent states:
                        $res = $GLOBALS['TYPO3_DB']->exec_SELECTquery(
                                                'sys_history.*,sys_log.userid',
@@ -786,7 +803,7 @@ class recordHistory {
 
                $out = $table.':'.$uid;
                if ($labelField = $TCA[$table]['ctrl']['label'])        {
-                       $record = t3lib_BEfunc::getRecordRaw($table, 'uid='.intval($uid));
+                       $record = $this->getRecord($table, $uid);
                        $out .= ' ('.t3lib_BEfunc::getRecordTitle($table, $record, TRUE).')';
                }
                return $out;
@@ -877,13 +894,115 @@ class recordHistory {
         * @return      [type]          ...
         */
        function resolveShUid() {
-               if (t3lib_div::_GP('sh_uid'))   {
-                       $sh_uid = t3lib_div::_GP('sh_uid');
+               if ($this->getArgument('sh_uid')) {
+                       $sh_uid = $this->getArgument('sh_uid');
                        $res = $GLOBALS['TYPO3_DB']->exec_SELECTquery('*','sys_history', 'uid='.intval($sh_uid));
                        $record = $GLOBALS['TYPO3_DB']->sql_fetch_assoc($res);
                        $this->element = $record['tablename'].':'.$record['recuid'];
                        $this->lastSyslogId = $record['sys_log_uid']-1;
                }
        }
+
+       /**
+        * Determines whether user has access to a page.
+        *
+        * @param string $table
+        * @param integer $uid
+        * @return boolean
+        */
+       protected function hasPageAccess($table, $uid) {
+               $uid = intval($uid);
+
+               if ($table === 'pages') {
+                       $pageId = $uid;
+               } else {
+                       $record = $this->getRecord($table, $uid);
+                       $pageId = $record['pid'];
+               }
+
+               if (!isset($this->pageAccessCache[$pageId])) {
+                       $this->pageAccessCache[$pageId] = t3lib_BEfunc::readPageAccess(
+                               $pageId, $this->getBackendUser()->getPagePermsClause(1)
+                       );
+               }
+
+               return ($this->pageAccessCache[$pageId] !== FALSE);
+       }
+
+       /**
+        * Determines whether user has access to a table.
+        *
+        * @param string $table
+        * @return boolean
+        */
+       protected function hasTableAccess($table) {
+               return $this->getBackendUser()->check('tables_select', $table);
+       }
+
+       /**
+        * Gets a database record.
+        *
+        * @param string $table
+        * @param integer $uid
+        * @return array|NULL
+        */
+       protected function getRecord($table, $uid) {
+               if (!isset($this->recordCache[$table][$uid])) {
+                       $this->recordCache[$table][$uid] = t3lib_BEfunc::getRecord($table, $uid, '*', '', FALSE);
+               }
+               return $this->recordCache[$table][$uid];
+       }
+
+       /**
+        * Gets the current backend user.
+        *
+        * @return t3lib_beUserAuth
+        */
+       protected function getBackendUser() {
+               return $GLOBALS['BE_USER'];
+       }
+
+       /**
+        * Fetches GET/POST arguments and sanitizes the values for
+        * the expected disposal. Invalid values will be converted
+        * to an empty string or an empty array.
+        *
+        * @param string $name Name of the argument
+        * @return array|string|integer
+        */
+       protected function getArgument($name) {
+               $value = t3lib_div::_GP($name);
+
+               switch ($name) {
+                       case 'element':
+                               if ($value !== '' && !preg_match('#^[a-z0-9_.]+:[0-9]+$#i', $value)) {
+                                       $value = '';
+                               }
+                               break;
+                       case 'rollbackFields':
+                       case 'revert':
+                               if ($value !== '' && !preg_match('#^[a-z0-9_.]+(:[0-9]+(:[a-z0-9_.]+)?)?$#i', $value)) {
+                                       $value = '';
+                               }
+                               break;
+                       case 'returnUrl':
+                               $value = t3lib_div::sanitizeLocalUrl($value);
+                               break;
+                       case 'diff':
+                       case 'highlight':
+                       case 'sh_uid':
+                               $value = intval($value);
+                               break;
+                       case 'settings':
+                               if (!is_array($value)) {
+                                       $value = array();
+                               }
+                               break;
+                       default:
+                               $value = '';
+               }
+
+               return $value;
+       }
 }
 ?>
\ No newline at end of file