From 155169419819c271cd3f22dec46b1a17cb8b3dd7 Mon Sep 17 00:00:00 2001
From: Pierre Goiffon <pierre.goiffon@combodo.com>
Date: Mon, 5 Oct 2020 14:31:26 +0200
Subject: [PATCH 1/3] =?UTF-8?q?N=C2=B03317=20Security=20hardening?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 application/ajaxwebpage.class.inc.php       | 23 +++++++++++----------
 application/csvpage.class.inc.php           |  5 +++--
 application/itopwebpage.class.inc.php       |  1 +
 application/loginwebpage.class.inc.php      |  8 +++----
 application/webpage.class.inc.php           |  1 +
 application/xmlpage.class.inc.php           |  3 ++-
 datamodels/2.x/combodo-db-tools/dbtools.php |  1 -
 datamodels/2.x/itop-backup/status.php       |  1 -
 datamodels/2.x/itop-config/config.php       |  1 -
 pages/UI.php                                |  1 -
 pages/UniversalSearch.php                   |  1 -
 pages/notifications.php                     |  1 -
 pages/run_query.php                         |  1 -
 pages/schema.php                            |  1 -
 webservices/export-v2.php                   |  4 ----
 15 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/application/ajaxwebpage.class.inc.php b/application/ajaxwebpage.class.inc.php
index f84d13918..b74c17db8 100644
--- a/application/ajaxwebpage.class.inc.php
+++ b/application/ajaxwebpage.class.inc.php
@@ -42,21 +42,22 @@ class ajax_page extends WebPage implements iTabbedPage
      */	  
 	function __construct($s_title)
     {
-		$sPrintable = utils::ReadParam('printable', '0');
-		$bPrintable = ($sPrintable == '1');
+	    $sPrintable = utils::ReadParam('printable', '0');
+	    $bPrintable = ($sPrintable == '1');
 
-        parent::__construct($s_title, $bPrintable);
-        $this->m_sReadyScript = "";
-		//$this->add_header("Content-type: text/html; charset=utf-8");
-		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
+	    parent::__construct($s_title, $bPrintable);
+	    $this->m_sReadyScript = "";
+	    //$this->add_header("Content-type: text/html; charset=utf-8");
+	    $this->add_header('Cache-control: no-cache, no-store, must-revalidate');
 	    $this->add_header('Pragma: no-cache');
 	    $this->add_header('Expires: 0');
-		$this->m_oTabs = new TabManager();
-		$this->sContentType = 'text/html';
-		$this->sContentDisposition = 'inline';
-		$this->m_sMenu = "";
+	    $this->add_header('X-Frame-Options: deny');
+	    $this->m_oTabs = new TabManager();
+	    $this->sContentType = 'text/html';
+	    $this->sContentDisposition = 'inline';
+	    $this->m_sMenu = "";
 
-		utils::InitArchiveMode();
+	    utils::InitArchiveMode();
     }
 
 	public function AddTabContainer($sTabContainer, $sPrefix = '')
diff --git a/application/csvpage.class.inc.php b/application/csvpage.class.inc.php
index 537c23194..25dc612a2 100644
--- a/application/csvpage.class.inc.php
+++ b/application/csvpage.class.inc.php
@@ -31,12 +31,13 @@ class CSVPage extends WebPage
 {
     function __construct($s_title)
     {
-        parent::__construct($s_title);
+	    parent::__construct($s_title);
 	    $this->add_header("Content-type: text/plain; charset=utf-8");
 	    $this->add_header('Cache-control: no-cache, no-store, must-revalidate');
 	    $this->add_header('Pragma: no-cache');
 	    $this->add_header('Expires: 0');
-		//$this->add_header("Content-Transfer-Encoding: binary");
+	    $this->add_header('X-Frame-Options: deny');
+	    //$this->add_header("Content-Transfer-Encoding: binary");
     }	
 
     public function output()
diff --git a/application/itopwebpage.class.inc.php b/application/itopwebpage.class.inc.php
index 19a3aa7cf..e459d8ae9 100644
--- a/application/itopwebpage.class.inc.php
+++ b/application/itopwebpage.class.inc.php
@@ -75,6 +75,7 @@ class iTopWebPage extends NiceWebPage implements iTabbedPage
 		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
 		$this->add_header('Pragma: no-cache');
 		$this->add_header('Expires: 0');
+		$this->add_header('X-Frame-Options: deny');
 		$this->add_linked_stylesheet("../css/jquery.treeview.css");
 		$this->add_linked_stylesheet("../css/jquery.autocomplete.css");
 		$this->add_linked_stylesheet("../css/jquery-ui-timepicker-addon.css");
diff --git a/application/loginwebpage.class.inc.php b/application/loginwebpage.class.inc.php
index 02b03f12b..ce42ee050 100644
--- a/application/loginwebpage.class.inc.php
+++ b/application/loginwebpage.class.inc.php
@@ -62,16 +62,16 @@ class LoginWebPage extends NiceWebPage
 	
 	public function __construct($sTitle = null)
 	{
-	    if($sTitle === null)
-        {
-            $sTitle = Dict::S('UI:Login:Title');
-        }
+		if ($sTitle === null) {
+			$sTitle = Dict::S('UI:Login:Title');
+		}
 
 		parent::__construct($sTitle);
 		$this->SetStyleSheet();
 		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
 		$this->add_header('Pragma: no-cache');
 		$this->add_header('Expires: 0');
+		$this->add_header('X-Frame-Options: deny');
 	}
 	
 	public function SetStyleSheet()
diff --git a/application/webpage.class.inc.php b/application/webpage.class.inc.php
index b75707c17..bcaf20327 100644
--- a/application/webpage.class.inc.php
+++ b/application/webpage.class.inc.php
@@ -358,6 +358,7 @@ class WebPage implements Page
 		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
 		$this->add_header('Pragma: no-cache');
 		$this->add_header('Expires: 0');
+		$this->add_header('X-Frame-Options: deny');
 	}
 
 	/**
diff --git a/application/xmlpage.class.inc.php b/application/xmlpage.class.inc.php
index 7a8cb47fc..70d5ba2dd 100644
--- a/application/xmlpage.class.inc.php
+++ b/application/xmlpage.class.inc.php
@@ -46,8 +46,9 @@ class XMLPage extends WebPage
 		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
 		$this->add_header('Pragma: no-cache');
 		$this->add_header('Expires: 0');
+		$this->add_header('X-Frame-Options: deny');
 		$this->add_header("Content-location: export.xml");
-	}	
+	}
 
 	public function output()
 	{
diff --git a/datamodels/2.x/combodo-db-tools/dbtools.php b/datamodels/2.x/combodo-db-tools/dbtools.php
index 4635eca09..32e22353e 100644
--- a/datamodels/2.x/combodo-db-tools/dbtools.php
+++ b/datamodels/2.x/combodo-db-tools/dbtools.php
@@ -570,7 +570,6 @@ try
 	$sPageId = 'db-tools';
 
 	$oP = new iTopWebPage($sPageTitle);
-	$this->m_oPage->add_header('X-Frame-Options: deny');
 	$oP->add_saas('env-'.utils::GetCurrentEnvironment().'/combodo-db-tools/default.scss');
 
 	$oP->add(
diff --git a/datamodels/2.x/itop-backup/status.php b/datamodels/2.x/itop-backup/status.php
index d33615680..feeeb9b1b 100644
--- a/datamodels/2.x/itop-backup/status.php
+++ b/datamodels/2.x/itop-backup/status.php
@@ -37,7 +37,6 @@ try
 {
 	$sTransactionId = utils::GetNewTransactionId();
 	$oP = new iTopWebPage(Dict::S('bkp-status-title'));
-	$oP->add_header('X-Frame-Options: deny');
 	$oP->set_base(utils::GetAbsoluteUrlAppRoot().'pages/');
 
 	$oP->add("<h1>".Dict::S('bkp-status-title')."</h1>");
diff --git a/datamodels/2.x/itop-config/config.php b/datamodels/2.x/itop-config/config.php
index de9bb584d..7e9f9231b 100644
--- a/datamodels/2.x/itop-config/config.php
+++ b/datamodels/2.x/itop-config/config.php
@@ -100,7 +100,6 @@ ApplicationMenu::CheckMenuIdEnabled('ConfigEditor');
 //$oAppContext = new ApplicationContext();
 
 $oP = new iTopWebPage(Dict::S('config-edit-title'));
-$oP->add_header('X-Frame-Options: deny');
 $oP->set_base(utils::GetAbsoluteUrlAppRoot().'pages/');
 $oP->add_linked_script(utils::GetCurrentModuleUrl().'/js/ace.js');
 $oP->add_linked_script(utils::GetCurrentModuleUrl().'/js/mode-php.js');
diff --git a/pages/UI.php b/pages/UI.php
index 367b4284d..f4222c540 100644
--- a/pages/UI.php
+++ b/pages/UI.php
@@ -1780,7 +1780,6 @@ EOF
 		///////////////////////////////////////////////////////////////////////////////////////////
 
 		default: // Menu node rendering (templates)
-		$oP->add_header('X-Frame-Options: deny');
 		ApplicationMenu::LoadAdditionalMenus();
 		$oMenuNode = ApplicationMenu::GetMenuNode(ApplicationMenu::GetMenuIndexById(ApplicationMenu::GetActiveNodeId()));
 		if (is_object($oMenuNode))
diff --git a/pages/UniversalSearch.php b/pages/UniversalSearch.php
index aeae6c3fb..04da4fe69 100644
--- a/pages/UniversalSearch.php
+++ b/pages/UniversalSearch.php
@@ -38,7 +38,6 @@ ApplicationMenu::CheckMenuIdEnabled('UniversalSearchMenu');
 $oAppContext = new ApplicationContext();
 
 $oP = new iTopWebPage(Dict::S('UI:UniversalSearchTitle'));
-$oP->add_header('X-Frame-Options: deny');
 $oP->add_linked_script("../js/json.js");
 $oP->add_linked_script("../js/forms-json-utils.js");
 $oP->add_linked_script("../js/wizardhelper.js");
diff --git a/pages/notifications.php b/pages/notifications.php
index 866c468da..384db331a 100644
--- a/pages/notifications.php
+++ b/pages/notifications.php
@@ -37,7 +37,6 @@ ApplicationMenu::CheckMenuIdEnabled("NotificationsMenu");
 // Main program
 //
 $oP = new iTopWebPage(Dict::S('Menu:NotificationsMenu+'));
-$oP->add_header('X-Frame-Options: deny');
 
 $oP->add('<div class="page_header" style="padding:0.5em;">');
 $oP->add('<h1>'.dict::S('UI:NotificationsMenu:Title').'</h1>');
diff --git a/pages/run_query.php b/pages/run_query.php
index b2e49f96f..2d9288c34 100644
--- a/pages/run_query.php
+++ b/pages/run_query.php
@@ -100,7 +100,6 @@ $oAppContext = new ApplicationContext();
 
 $oP = new iTopWebPage(Dict::S('UI:RunQuery:Title'));
 $oP->SetBreadCrumbEntry('ui-tool-runquery', Dict::S('Menu:RunQueriesMenu'), Dict::S('Menu:RunQueriesMenu+'), '', utils::GetAbsoluteUrlAppRoot().'images/wrench.png');
-$oP->add_header('X-Frame-Options: deny');
 
 // Main program
 $sExpression = utils::ReadParam('expression', '', false, 'raw_data');
diff --git a/pages/schema.php b/pages/schema.php
index 64fc0e97a..8c69eb18d 100644
--- a/pages/schema.php
+++ b/pages/schema.php
@@ -1074,7 +1074,6 @@ $operation = utils::ReadParam('operation', '');
 
 $oPage = new iTopWebPage(Dict::S('UI:Schema:Title'));
 $oPage->no_cache();
-$oPage->add_header('X-Frame-Options: deny');
 
 $oPage->SetBreadCrumbEntry('ui-tool-datamodel', Dict::S('Menu:DataModelMenu'), Dict::S('Menu:DataModelMenu+'), '', utils::GetAbsoluteUrlAppRoot().'images/wrench.png');
 $oPage->add_script(
diff --git a/webservices/export-v2.php b/webservices/export-v2.php
index 3046766b0..ddd355b3b 100644
--- a/webservices/export-v2.php
+++ b/webservices/export-v2.php
@@ -352,7 +352,6 @@ function InteractiveShell($sExpression, $sQueryId, $sFormat, $sFileName, $sMode)
 	if ($sMode == 'dialog')
 	{
 		$oP = new ajax_page('');
-		$oP->add_header('X-Frame-Options: deny');
 		$oP->add('<div id="interactive_export_dlg">');
 		$sExportBtnLabel = json_encode(Dict::S('UI:Button:Export'));
 		$sJSTitle = json_encode(htmlentities(utils::ReadParam('dialog_title', '', false, 'raw_data'), ENT_QUOTES, 'UTF-8'));
@@ -378,7 +377,6 @@ EOF
 	else
 	{
 		$oP = new iTopWebPage('iTop Export');
-		$oP->add_header('X-Frame-Options: deny');
 		$oP->SetBreadCrumbEntry('ui-tool-export', Dict::S('Menu:ExportMenu'), Dict::S('Menu:ExportMenu+'), '', utils::GetAbsoluteUrlAppRoot().'images/wrench.png');
 	}
 	
@@ -753,7 +751,6 @@ try
 		else
 		{
 			$oP = new ajax_page('iTop export');
-			$oP->add_header('X-Frame-Options: deny');
 			$oP->SetContentType($oExporter->GetMimeType());
 		}
 		DoExport($oP, $oExporter, false);
@@ -763,7 +760,6 @@ try
 catch (BulkExportMissingParameterException $e)
 {
 	$oP = new ajax_page('iTop Export');
-	$oP->add_header('X-Frame-Options: deny');
 	$oP->add($e->getMessage());
 	Usage($oP);
 	$oP->output();

From 090119147cf6c80879e1a268de094c9b5d64ee46 Mon Sep 17 00:00:00 2001
From: Pierre Goiffon <pierre.goiffon@combodo.com>
Date: Mon, 5 Oct 2020 14:36:28 +0200
Subject: [PATCH 2/3] :art: PHP formatting

---
 application/ajaxwebpage.class.inc.php | 48 +++++++++++++--------------
 application/csvpage.class.inc.php     | 19 +++++------
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/application/ajaxwebpage.class.inc.php b/application/ajaxwebpage.class.inc.php
index b74c17db8..3c48108ab 100644
--- a/application/ajaxwebpage.class.inc.php
+++ b/application/ajaxwebpage.class.inc.php
@@ -30,35 +30,35 @@ class ajax_page extends WebPage implements iTabbedPage
 {
     /**
      * Jquery style ready script
-     * @var Hash     
-     */	  
+     * @var Hash
+     */
 	protected $m_sReadyScript;
 	protected $m_oTabs;
 	private $m_sMenu; // If set, then the menu will be updated
-	
-    /**
-     * constructor for the web page
-     * @param string $s_title Not used
-     */	  
-	function __construct($s_title)
-    {
-	    $sPrintable = utils::ReadParam('printable', '0');
-	    $bPrintable = ($sPrintable == '1');
 
-	    parent::__construct($s_title, $bPrintable);
-	    $this->m_sReadyScript = "";
-	    //$this->add_header("Content-type: text/html; charset=utf-8");
-	    $this->add_header('Cache-control: no-cache, no-store, must-revalidate');
-	    $this->add_header('Pragma: no-cache');
-	    $this->add_header('Expires: 0');
-	    $this->add_header('X-Frame-Options: deny');
-	    $this->m_oTabs = new TabManager();
-	    $this->sContentType = 'text/html';
-	    $this->sContentDisposition = 'inline';
-	    $this->m_sMenu = "";
+	/**
+	 * constructor for the web page
+	 *
+	 * @param string $s_title Not used
+	 */
+	function __construct($s_title) {
+		$sPrintable = utils::ReadParam('printable', '0');
+		$bPrintable = ($sPrintable == '1');
 
-	    utils::InitArchiveMode();
-    }
+		parent::__construct($s_title, $bPrintable);
+		$this->m_sReadyScript = "";
+		//$this->add_header("Content-type: text/html; charset=utf-8");
+		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
+		$this->add_header('Pragma: no-cache');
+		$this->add_header('Expires: 0');
+		$this->add_header('X-Frame-Options: deny');
+		$this->m_oTabs = new TabManager();
+		$this->sContentType = 'text/html';
+		$this->sContentDisposition = 'inline';
+		$this->m_sMenu = "";
+
+		utils::InitArchiveMode();
+	}
 
 	public function AddTabContainer($sTabContainer, $sPrefix = '')
 	{
diff --git a/application/csvpage.class.inc.php b/application/csvpage.class.inc.php
index 25dc612a2..4a12a7520 100644
--- a/application/csvpage.class.inc.php
+++ b/application/csvpage.class.inc.php
@@ -29,16 +29,15 @@ require_once(APPROOT."/application/webpage.class.inc.php");
 
 class CSVPage extends WebPage
 {
-    function __construct($s_title)
-    {
-	    parent::__construct($s_title);
-	    $this->add_header("Content-type: text/plain; charset=utf-8");
-	    $this->add_header('Cache-control: no-cache, no-store, must-revalidate');
-	    $this->add_header('Pragma: no-cache');
-	    $this->add_header('Expires: 0');
-	    $this->add_header('X-Frame-Options: deny');
-	    //$this->add_header("Content-Transfer-Encoding: binary");
-    }	
+	function __construct($s_title) {
+		parent::__construct($s_title);
+		$this->add_header("Content-type: text/plain; charset=utf-8");
+		$this->add_header('Cache-control: no-cache, no-store, must-revalidate');
+		$this->add_header('Pragma: no-cache');
+		$this->add_header('Expires: 0');
+		$this->add_header('X-Frame-Options: deny');
+		//$this->add_header("Content-Transfer-Encoding: binary");
+	}
 
     public function output()
     {

From 1f5375731864bd78a1b692b18dd8e7abc9a2c98b Mon Sep 17 00:00:00 2001
From: Eric <eric.espie@combodo.com>
Date: Tue, 18 Aug 2020 17:21:25 +0200
Subject: [PATCH 3/3] =?UTF-8?q?N=C2=B03248=20-=20code=20hardening=20(cherr?=
 =?UTF-8?q?y=20picked=20from=20commit=206a2593374433a72b3d23832db04f48b703?=
 =?UTF-8?q?854e4f)=20(cherry=20picked=20from=20commit=20f74c78d61c4bd20439?=
 =?UTF-8?q?4a4c216e6fad0e1e199ddd)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 application/transaction.class.inc.php | 9 ++++++++-
 application/utils.inc.php             | 3 ++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/application/transaction.class.inc.php b/application/transaction.class.inc.php
index ec073cd52..0e4ca4be1 100644
--- a/application/transaction.class.inc.php
+++ b/application/transaction.class.inc.php
@@ -233,7 +233,14 @@ class privUITransactionFile
 	 */
 	public static function IsTransactionValid($id, $bRemoveTransaction = true)
 	{
-		$sFilepath = APPROOT.'data/transactions/'.$id;
+		// Constraint the transaction file within APPROOT.'data/transactions'
+		$sTransactionDir = realpath(APPROOT.'data/transactions');
+		$sFilepath = utils::RealPath($sTransactionDir.'/'.$id, $sTransactionDir);
+		if (($sFilepath === false) || (strlen($sTransactionDir) == strlen($sFilepath)))
+		{
+			return false;
+		}
+
 		clearstatcache(true, $sFilepath);
 		$bResult = file_exists($sFilepath);
 		if ($bResult)
diff --git a/application/utils.inc.php b/application/utils.inc.php
index 08d816459..5c9f34817 100644
--- a/application/utils.inc.php
+++ b/application/utils.inc.php
@@ -308,6 +308,7 @@ class utils
 			case 'context_param':
 			case 'parameter':
 			case 'field_name':
+			case 'transaction_id':
 				if (is_array($value))
 				{
 					$retValue = array();
@@ -2086,7 +2087,7 @@ class utils
 	 * @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/'
 	 *
-	 * @return bool false if path :
+	 * @return bool|string false if path :
 	 *      * invalid
 	 *      * not allowed
 	 *      * not contained in base path