N°2538 enforce generic method to check path validity

Now uses realpath() and StartsWith
This commit is contained in:
Pierre Goiffon
2019-10-16 10:43:01 +02:00
parent 29c30c1f89
commit 607d355c61
4 changed files with 58 additions and 12 deletions

View File

@@ -2099,18 +2099,29 @@ class utils
} }
/** /**
* Checks that path does not contains illegal characters, like '../' * @param string $sPath for example '/var/www/html/itop/data/backups/manual/itop_27-2019-10-03_15_35.tar.gz'
* @param string $sBasePath for example '/var/www/html/itop/data/'
* *
* @param string $sPath * @return bool false if path :
* * invalid
* * not allowed
* * not contained in base path
* Otherwise return the real path (see realpath())
* *
* @return bool true if path is allowed, false otherwise * @since 2.7.0 N°2538
*
* @since 2.7.0
*/ */
final public static function IsAllowedPath($sPath) final public static function RealPath($sPath, $sBasePath)
{ {
$sPathNoDotDotPattern = "/^((?![\/\\\\]\.\.[\/\\\\]).)*$/"; $sFileRealPath = realpath($sPath);
if ($sFileRealPath === false)
{
return false;
}
if (!self::StartsWith($sFileRealPath, $sBasePath))
{
return false;
}
return preg_match($sPathNoDotDotPattern, $sPath) == 1; return $sFileRealPath;
} }
} }

View File

@@ -1,6 +1,6 @@
<?php <?php
define('APPROOT', dirname(__FILE__).'/'); define('APPROOT', dirname(__FILE__).DIRECTORY_SEPARATOR);
define('APPCONF', APPROOT.'conf/'); define('APPCONF', APPROOT.'conf/');
define('ITOP_DEFAULT_ENV', 'production'); define('ITOP_DEFAULT_ENV', 'production');
define('MAINTENANCE_MODE_FILE', APPROOT.'data/.maintenance'); define('MAINTENANCE_MODE_FILE', APPROOT.'data/.maintenance');

View File

@@ -179,13 +179,13 @@ EOF
$sFile = utils::ReadParam('file', '', false, 'raw_data'); $sFile = utils::ReadParam('file', '', false, 'raw_data');
$oBackup = new DBBackupScheduled(); $oBackup = new DBBackupScheduled();
$sBackupDir = APPROOT.'data/backups/'; $sBackupDir = APPROOT.'data/backups/';
if (utils::IsAllowedPath($sBackupDir.$sFile)) if ($sBackupFilePath = utils::RealPath($sBackupDir.$sFile, $sBackupDir))
{ {
$oBackup->DownloadBackup($sBackupDir.$sFile); $oBackup->DownloadBackup($sBackupFilePath);
} }
else else
{ {
throw new InvalidParameterException('Invalid file path'); throw new CoreUnexpectedValue('Invalid file path');
} }
break; break;
} }

View File

@@ -50,4 +50,39 @@ class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase
[false, 1024, 2048], [false, 1024, 2048],
]; ];
} }
/**
* @dataProvider realPathDataProvider
*/
public function testRealPath($sPath, $sBasePath, $expected)
{
$this->assertSame($expected, utils::RealPath($sPath, $sBasePath));
}
public function realPathDataProvider()
{
parent::setUp(); // if not called, APPROOT won't be defined :(
$sItopRootPath = APPROOT;
$sSep = DIRECTORY_SEPARATOR;
return [
'licence.txt' => [$sItopRootPath.'license.txt', $sItopRootPath, $sItopRootPath.'license.txt'],
'unexisting file' => [$sItopRootPath.'license_DOES_NOT_EXIST.txt', $sItopRootPath, false],
'/license.txt' => [$sItopRootPath.$sSep.'license.txt', $sItopRootPath, $sItopRootPath.'license.txt'],
'%2flicense.txt' => [$sItopRootPath.'%2flicense.txt', $sItopRootPath, false],
'../license.txt' => [$sItopRootPath.'..'.$sSep.'license.txt', $sItopRootPath, false],
'%2e%2e%2flicense.txt' => [$sItopRootPath.'%2e%2e%2flicense.txt', $sItopRootPath, false],
'application/utils.inc.php with basepath=APPROOT' => [
$sItopRootPath.'application/utils.inc.php',
$sItopRootPath,
$sItopRootPath.'application'.$sSep.'utils.inc.php',
],
'application/utils.inc.php with basepath=APPROOT/application' => [
$sItopRootPath.'application/utils.inc.php',
$sItopRootPath.'application',
$sItopRootPath.'application'.$sSep.'utils.inc.php',
],
];
}
} }