Fixed bug #15898: It is (still) possible to download arbitrary files …

…through the jumpurl feature (thanks to Helmut Hummel and Marcus Krause) git-svn-id: https://svn.typo3.org/TYPO3v4/Core/branches/TYPO3_4-2@8979 709f56b5-9817-0410-a4d7-c38de5d9e867
TYPO3 · Oct 6, 2010 · 687b671 · 687b671
1 parent 893ca75
commit 687b671
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 18 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -5,6 +5,7 @@
 	* Fixed bug #15728: Extension Manager allows to download arbitrary files beyond PATH_site or rootpath (thanks to Marcus Krause)
 	* Fixed bug #15729: Sysext setup's user simulation is susceptible to XSS (thanks to Marcus Krause)
 	* Fixed bug #15733: Admin Panel is susceptible to XSS (thanks to Helmut Hummel)
+	* Fixed bug #15898: It is (still) possible to download arbitrary files through the jumpurl feature (thanks to Helmut Hummel and Marcus Krause)
 
 2010-09-24  Steffen Gebert  <steffen@steffen-gebert.de>
 

diff --git a/typo3/sysext/cms/tslib/class.tslib_content.php b/typo3/sysext/cms/tslib/class.tslib_content.php
@@ -3981,6 +3981,7 @@ function filelink($theValue, $conf)	{
 	function locDataJU($jumpUrl,$conf)	{
 		$fI = pathinfo($jumpUrl);
 		$mimetype='';
+		$mimetypeValue = '';
 		if ($fI['extension'])	{
 			$mimeTypes = t3lib_div::trimExplode(',',$conf['mimeTypes'],1);
 			reset($mimeTypes);
@@ -3996,12 +3997,9 @@ function locDataJU($jumpUrl,$conf)	{
 		$locationData = $GLOBALS['TSFE']->id.':'.$this->currentRecord;
 		$rec='&locationData='.rawurlencode($locationData);
 		$hArr = array(
-			$jumpUrl,
-			$locationData,
-			$mimetypeValue,
-			$GLOBALS['TSFE']->TYPO3_CONF_VARS['SYS']['encryptionKey']
+			$jumpUrl, $locationData, $mimetypeValue
 		);
-		$juHash='&juHash='.t3lib_div::shortMD5(serialize($hArr));
+		$juHash = '&juHash=' . t3lib_div::hmac(serialize($hArr));
 		return '&juSecure=1'.$mimetype.$rec.$juHash;
 	}
 

diff --git a/typo3/sysext/cms/tslib/class.tslib_fe.php b/typo3/sysext/cms/tslib/class.tslib_fe.php
@@ -2520,31 +2520,31 @@ function checkJumpUrlReferer()	{
 	function jumpUrl()	{
 		if ($this->jumpurl)	{
 			if (t3lib_div::_GP('juSecure'))	{
-				$locationData = t3lib_div::_GP('locationData');
-				$mimeType = t3lib_div::_GP('mimeType');
+				$locationData = (string)t3lib_div::_GP('locationData');
+				$mimeType = (string)t3lib_div::_GP('mimeType');  // Need a type cast here because mimeType is optional!
 
 				$hArr = array(
 					$this->jumpurl,
-					t3lib_div::_GP('locationData'),
-					t3lib_div::_GP('mimeType'),
-					$this->TYPO3_CONF_VARS['SYS']['encryptionKey']
+					$locationData,
+					$mimeType
 				);
-				$calcJuHash=t3lib_div::shortMD5(serialize($hArr));
-				$juHash = t3lib_div::_GP('juHash');
-				if ($juHash == $calcJuHash)	{
+				$calcJuHash = t3lib_div::hmac(serialize($hArr));
+				$juHash = (string)t3lib_div::_GP('juHash');
+				if ($juHash === $calcJuHash) {
 					if ($this->locDataCheck($locationData))	{
 						$this->jumpurl = rawurldecode($this->jumpurl);	// 211002 - goes with cObj->filelink() rawurlencode() of filenames so spaces can be allowed.
 							// Deny access to files that match TYPO3_CONF_VARS[SYS][fileDenyPattern] and whose parent directory is typo3conf/ (there could be a backup file in typo3conf/ which does not match against the fileDenyPattern)
-						if (t3lib_div::verifyFilenameAgainstDenyPattern($this->jumpurl) && basename(dirname($this->jumpurl)) !== 'typo3conf') {
-							if (@is_file($this->jumpurl)) {
+						$absoluteFileName = t3lib_div::getFileAbsFileName(t3lib_div::resolveBackPath($this->jumpurl), FALSE);
+						if (t3lib_div::isAllowedAbsPath($absoluteFileName) && t3lib_div::verifyFilenameAgainstDenyPattern($absoluteFileName) && !t3lib_div::isFirstPartOfStr($absoluteFileName, PATH_site . 'typo3conf')) {
+							if (@is_file($absoluteFileName)) {
 								$mimeType = $mimeType ? $mimeType : 'application/octet-stream';
 								header('Cache-Control: must-revalidate, post-check=0, pre-check=0');
 								header('Content-Type: '.$mimeType);
-								header('Content-Disposition: attachment; filename='.basename($this->jumpurl));
-								readfile($this->jumpurl);
+								header('Content-Disposition: attachment; filename="'.basename($absoluteFileName) . '"');
+								readfile($absoluteFileName);
 								exit;
 							} else die('jumpurl Secure: "'.$this->jumpurl.'" was not a valid file!');
-						} else die('jumpurl Secure: The requested file type was not allowed to be accessed through jumpUrl (fileDenyPattern)!');
+						} else die('jumpurl Secure: The requested file was not allowed to be accessed through jumpUrl (path or file not allowed)!');
 					} else die('jumpurl Secure: locationData, '.$locationData.', was not accessible.');
 				} else die('jumpurl Secure: Calculated juHash did not match the submitted juHash.');
 			} else {