From 034ca26d01e0edeb0ef81a37d3638a396da1f63d Mon Sep 17 00:00:00 2001
From: "denis.flaven@combodo.com" <denis.flaven@combodo.com>
Date: Mon, 3 Apr 2023 11:54:01 +0200
Subject: [PATCH] Do NOT backup unsafe files.

---
 setup/backup.class.inc.php      | 18 +++++++++++++-----
 test/setup/DBBackupDataTest.php | 23 ++++++++++++++++++-----
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/setup/backup.class.inc.php b/setup/backup.class.inc.php
index a8d6f6849..92bc7b2cc 100644
--- a/setup/backup.class.inc.php
+++ b/setup/backup.class.inc.php
@@ -205,11 +205,12 @@ class DBBackup
 	 *
 	 * @param string $sSourceConfigFile
 	 * @param string $sTmpFolder
+	 * @param bool $bSkipSQLDumpForTesting 
 	 *
 	 * @return array list of files to archive
 	 * @throws \Exception
 	 */
-	protected function PrepareFilesToBackup($sSourceConfigFile, $sTmpFolder)
+	protected function PrepareFilesToBackup($sSourceConfigFile, $sTmpFolder, $bSkipSQLDumpForTesting = false)
 	{
 		$aRet = array();
 		if (is_dir($sTmpFolder))
@@ -250,7 +251,11 @@ class DBBackup
 		$aExtraFiles = MetaModel::GetModuleSetting('itop-backup', 'extra_files', []);
 		foreach($aExtraFiles as $sExtraFileOrDir)
 		{
-			$sExtraFullPath = APPROOT.'/'.$sExtraFileOrDir;
+			$sExtraFullPath = realpath(APPROOT.'/'.$sExtraFileOrDir);
+			if (strncmp(APPROOT, $sExtraFullPath, strlen(APPROOT)) !== 0)
+			{
+				throw new Exception("Backup: Aborting, resource '$sExtraFileOrDir'. Considered as UNSAFE because not inside the iTop directory.");
+			}
 			if (is_dir($sExtraFullPath))
 			{
 				$sFile = $sTmpFolder.'/'.$sExtraFileOrDir;
@@ -267,9 +272,12 @@ class DBBackup
 				$aRet[] = $sFile;
 			}
 		}
-		$sDataFile = $sTmpFolder.'/itop-dump.sql';
-		$this->DoBackup($sDataFile);
-		$aRet[] = $sDataFile;
+		if (!$bSkipSQLDumpForTesting)
+		{
+			$sDataFile = $sTmpFolder.'/itop-dump.sql';
+			$this->DoBackup($sDataFile);
+			$aRet[] = $sDataFile;
+		}
 
 		return $aRet;
 	}
diff --git a/test/setup/DBBackupDataTest.php b/test/setup/DBBackupDataTest.php
index aed8fe73c..af9595445 100644
--- a/test/setup/DBBackupDataTest.php
+++ b/test/setup/DBBackupDataTest.php
@@ -18,7 +18,7 @@ class DBBackupDataTest extends ItopDataTestCase
 	/**
 	 * @dataProvider prepareFilesToBackupProvider
 	 */
-	public function testPrepareFilesToBackup($aExtraFiles)
+	public function testPrepareFilesToBackup(array $aExtraFiles, bool $bUnsafeFileException)
 	{
 		$sTmpDir = sys_get_temp_dir().'/testPrepareFilesToBackup-'.time();
 		$oBackup = new DBBackup(MetaModel::GetConfig());
@@ -33,11 +33,14 @@ class DBBackupDataTest extends ItopDataTestCase
 			}
 		}
 		
-		$aFiles = $this->InvokeNonPublicMethod('DBBackup', 'PrepareFilesToBackup', $oBackup, [APPROOT.'/conf/production/config-itop.php', $sTmpDir]);
+		if ($bUnsafeFileException)
+		{
+			$this->expectExceptionMessage("Backup: Aborting, resource '$sExtraFile'. Considered as UNSAFE because not inside the iTop directory.");
+		}
+		$aFiles = $this->InvokeNonPublicMethod('DBBackup', 'PrepareFilesToBackup', $oBackup, [APPROOT.'/conf/production/config-itop.php', $sTmpDir, true]);
 		SetupUtils::rrmdir($sTmpDir);
 		$aExpectedFiles = [
 			$sTmpDir.'/config-itop.php',
-			$sTmpDir.'/itop-dump.sql',
 		];
 		foreach($aExtraFiles as $sRelFile => $bExists)
 		{
@@ -59,9 +62,19 @@ class DBBackupDataTest extends ItopDataTestCase
 			}
 		}
 	}
+	
+	function prepareFilesToBackupProvider()
+	{
+		return [
+			'no_extra_file' => ['aExtraFiles' => [], false],
+			'one_extra_file' => ['aExtraFiles' => ['foo.txt' => true], false],
+			'three_extra_file_and_dir' => ['aExtraFiles' => ['foo.txt' => true, 'gabu/zomeu.xml' => true, 'meuh.html' => true], false],
+			'one_unsafe_file' => ['aExtraFiles' => ['../foo.txt' => true], true],
+		];
+	}
 
 	/**
-	 * @dataProvider prepareFilesToBackupProvider
+	 * @dataProvider restoreListExtraFilesProvider
 	 */
 	function testRestoreListExtraFiles($aFilesToCreate, $aExpectedRelativeExtraFiles)
 	{
@@ -93,7 +106,7 @@ class DBBackupDataTest extends ItopDataTestCase
 		SetupUtils::rrmdir($sTmpDir);
 	}
 
-	function prepareFilesToBackupProvider()
+	function restoreListExtraFilesProvider()
 	{
 		return [
 			'no extra file' => ['aFilesToCreate' => ['config-itop.php', 'itop-dump.sql', 'delta.xml'], 'aExpectedExtraFiles' => []],