From 43daa2ef088bf928a2386fa19324628c3f19b807 Mon Sep 17 00:00:00 2001 From: Eric Date: Thu, 27 May 2021 09:29:50 +0200 Subject: [PATCH] =?UTF-8?q?=20N=C2=B03952=20-=20code=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/ajax.dataloader.php | 10 +--- setup/index.php | 19 +++++--- setup/setuputils.class.inc.php | 69 ++++++++++++++++++++++++++++ setup/wizardcontroller.class.inc.php | 8 ++-- setup/wizardsteps.class.inc.php | 12 +---- 5 files changed, 91 insertions(+), 27 deletions(-) diff --git a/setup/ajax.dataloader.php b/setup/ajax.dataloader.php index 127873410..fdc189114 100644 --- a/setup/ajax.dataloader.php +++ b/setup/ajax.dataloader.php @@ -147,12 +147,7 @@ header("Expires: Fri, 17 Jul 1970 05:00:00 GMT"); // Date in the past $sOperation = Utils::ReadParam('operation', ''); try { - $sAuthent = utils::ReadParam('authent', '', false, 'raw_data'); - if (!file_exists(APPROOT.'data/setup/authent') || $sAuthent !== file_get_contents(APPROOT.'data/setup/authent')) - { - throw new SecurityException('Setup operations are not allowed outside of the setup'); - SetupPage::log_error("Setup operations are not allowed outside of the setup"); - } + SetupUtils::CheckSetupToken(); switch($sOperation) { @@ -199,7 +194,6 @@ catch(Exception $e) { header("HTTP/1.0 500 Internal server error."); echo "

An error happened while processing the installation:

\n"; - echo '

'.$e."

\n"; SetupPage::log_error("An error happened while processing the installation: ".$e); } @@ -207,7 +201,7 @@ if (function_exists('memory_get_peak_usage')) { if ($sOperation == 'file') { - SetupPage::log_info("loading file '$sFileName', peak memory usage. ".memory_get_peak_usage()); + SetupPage::log_info("loading file peak memory usage. ".memory_get_peak_usage()); } else { diff --git a/setup/index.php b/setup/index.php index cc2cae1a5..ea08bb785 100644 --- a/setup/index.php +++ b/setup/index.php @@ -30,6 +30,7 @@ require_once(APPROOT.'/setup/setuppage.class.inc.php'); require_once(APPROOT.'/setup/wizardcontroller.class.inc.php'); require_once(APPROOT.'/setup/wizardsteps.class.inc.php'); +session_start(); clearstatcache(); // Make sure we know what we are doing ! // Set a long (at least 4 minutes) execution time for the setup to avoid timeouts during this phase ini_set('max_execution_time', max(240, ini_get('max_execution_time'))); @@ -41,16 +42,14 @@ date_default_timezone_set('Europe/Paris'); // Just to avoid a warning if the tim ///////////////////////////////////////////////////////////////////// // Fake functions to protect the first run of the installer // in case the PHP JSON module is not installed... -if (!function_exists('json_encode')) -{ +if (!function_exists('json_encode')) { function json_encode($value, $options = null) { return '[]'; } } -if (!function_exists('json_decode')) -{ - function json_decode($json, $assoc=null) +if (!function_exists('json_decode')) { + function json_decode($json, $assoc = null) { return array(); } @@ -58,4 +57,12 @@ if (!function_exists('json_decode')) ///////////////////////////////////////////////////////////////////// $oWizard = new WizardController('WizStepWelcome'); -$oWizard->Run(); +//N°3952 +if (SetupUtils::IsSessionSetupTokenValid()) { + // Normal operation + $oWizard->Run(); +} else { + // Force initializing the setup + $oWizard->Start(); + SetupUtils::CreateSetupToken(); +} diff --git a/setup/setuputils.class.inc.php b/setup/setuputils.class.inc.php index 7b918abbb..9ba7c82ea 100644 --- a/setup/setuputils.class.inc.php +++ b/setup/setuputils.class.inc.php @@ -1826,6 +1826,75 @@ EOF { return APPROOT.'log/setup-queries-'.strftime('%Y-%m-%d_%H_%M').'.sql'; } + + /** + * Create and store Setup authentication token + * + * @return string token + */ + public final static function CreateSetupToken() + { + if (!is_dir(APPROOT.'data')) + { + mkdir(APPROOT.'data'); + } + if (!is_dir(APPROOT.'data/setup')) + { + mkdir(APPROOT.'data/setup'); + } + $sUID = hash('sha256', rand()); + file_put_contents(APPROOT.'data/setup/authent', $sUID); + $_SESSION['setup_token'] = $sUID; + return $sUID; + } + + /** + * Verify Setup authentication token (from the request parameter 'authent') + * + * @param bool $bRemoveToken + * + * @throws \SecurityException + */ + public final static function CheckSetupToken($bRemoveToken = false) + { + $sAuthent = utils::ReadParam('authent', '', false, 'raw_data'); + $sTokenFile = APPROOT.'data/setup/authent'; + if (!file_exists($sTokenFile) || $sAuthent !== file_get_contents($sTokenFile)) + { + throw new SecurityException('Setup operations are not allowed outside of the setup'); + } + if ($bRemoveToken) + { + @unlink($sTokenFile); + } + } + + /** + * Check setup transaction and create a new one if necessary + * + * @return bool + */ + public static function IsSessionSetupTokenValid() + { + if (isset($_SESSION['setup_token'])) { + $sAuth = $_SESSION['setup_token']; + $sTokenFile = APPROOT.'data/setup/authent'; + if (file_exists($sTokenFile) && $sAuth === file_get_contents($sTokenFile)) { + return true; + } + } + + return false; + } + + public static function EraseSetupToken() + { + $sTokenFile = APPROOT.'data/setup/authent'; + if (is_file($sTokenFile)) { + unlink($sTokenFile); + } + unset($_SESSION['setup_token']); + } } /** diff --git a/setup/wizardcontroller.class.inc.php b/setup/wizardcontroller.class.inc.php index ac991e4ce..377092be6 100644 --- a/setup/wizardcontroller.class.inc.php +++ b/setup/wizardcontroller.class.inc.php @@ -105,7 +105,7 @@ class WizardController /** * Starts the wizard by displaying it in its initial state */ - protected function Start() + public function Start() { $sCurrentStepClass = $this->sInitialStepClass; $oStep = new $sCurrentStepClass($this, $this->sInitialState); @@ -121,7 +121,7 @@ class WizardController $sCurrentState = utils::ReadParam('_state', $this->sInitialState); /** @var \WizardStep $oStep */ $oStep = new $sCurrentStepClass($this, $sCurrentState); - if ($oStep->ValidateParams($sCurrentState)) + if ($oStep->ValidateParams()) { $this->PushStep(array('class' => $sCurrentStepClass, 'state' => $sCurrentState)); $aPossibleSteps = $oStep->GetPossibleSteps(); @@ -173,6 +173,7 @@ class WizardController // The configuration file already exists if (!is_writable($sConfigFile)) { + SetupUtils::EraseSetupToken(); $sRelativePath = utils::GetConfigFilePathRelative(); $oP = new SetupPage('Installation Cannot Continue'); $oP->add("

Fatal error

\n"); @@ -180,7 +181,8 @@ class WizardController $oP->p("The wizard cannot modify the configuration file for you. If you want to upgrade ".ITOP_APPLICATION.", make sure that the file '".$sRelativePath."' can be modified by the web server."); $oP->p(''); $oP->output(); - return; + // Prevent token creation + exit; } } } diff --git a/setup/wizardsteps.class.inc.php b/setup/wizardsteps.class.inc.php index 3efdddd69..2936be7f0 100644 --- a/setup/wizardsteps.class.inc.php +++ b/setup/wizardsteps.class.inc.php @@ -57,16 +57,7 @@ class WizStepWelcome extends WizardStep public function ProcessParams($bMoveForward = true) { - if (!is_dir(APPROOT.'data')) - { - mkdir(APPROOT.'data'); - } - if (!is_dir(APPROOT.'data/setup')) - { - mkdir(APPROOT.'data/setup'); - } - $sUID = hash('sha256', rand()); - file_put_contents(APPROOT.'data/setup/authent', $sUID); + $sUID = SetupUtils::CreateSetupToken(); $this->oWizard->SetParameter('authent', $sUID); return array('class' => 'WizStepInstallOrUpgrade', 'state' => ''); } @@ -2643,6 +2634,7 @@ class WizStepDone extends WizardStep $oPage->add(''); $sForm = addslashes($sForm); $oPage->add_ready_script("$('#wiz_form').after('$sForm');"); + SetupUtils::EraseSetupToken(); } public function CanMoveForward()