N°9654 - Reduce surface attack on config file rights during setup (#932)

This commit is contained in:
Lenaick
2026-06-09 16:56:58 +02:00
committed by GitHub
parent f48efbbd03
commit 733eea8a78
15 changed files with 46 additions and 83 deletions

View File

@@ -85,11 +85,8 @@ class DataFeatureRemovalController extends Controller
{
$aParams = [];
try {
//from setup wizard/mtp
SetupUtils::CheckSetupToken();
SetupUtils::EraseSetupToken();
} catch (SecurityException $e) {
//from setup wizard/mtp
if (!SetupUtils::IsSessionSetupTokenValid()) {
//from same module
$this->ValidateTransactionId();
}
@@ -184,7 +181,6 @@ class DataFeatureRemovalController extends Controller
$aParams['aSetupParams'] = [
"_class" => "WizStepLandingBeforeAudit",
"operation" => "next",
"_params[authent]" => SetupUtils::CreateSetupToken(),
];
foreach ($aHiddenInputs as $sInputName => $sInputValue) {
@@ -200,6 +196,10 @@ class DataFeatureRemovalController extends Controller
$aParams['bDeletionNeeded'] = ($aParams['iQueryCount'] > 0);
Session::Set('aDeletionExecutionSummary', serialize($this->aDeletionExecutionSummary));
if (!$aParams['bHasDeletionNeeded']) {
SetupUtils::CreateSetupToken();
}
$this->DisplayPage($aParams, 'AnalysisResult');
}

View File

@@ -35,6 +35,7 @@
* 'percent': integer 0..100 the percentage of completion once the file has been loaded
*/
use Combodo\iTop\Application\Helper\Session;
use Combodo\iTop\Application\WebPage\AjaxPage;
$bBypassMaintenance = true; // Reset maintenance mode in case of problem
@@ -129,7 +130,10 @@ header("Expires: Fri, 17 Jul 1970 05:00:00 GMT"); // Date in the past
*/
$sOperation = utils::ReadParam('operation', '');
try {
SetupUtils::CheckSetupToken();
Session::Start();
if (!SetupUtils::IsSessionSetupTokenValid()) {
throw new SecurityException("Invalid session token");
}
switch ($sOperation) {
case 'async_action':
@@ -150,14 +154,7 @@ try {
/** @var WizardStep $oStep */
$oStep = new $sClass($oDummyController, $sState);
$sConfigFile = utils::GetConfigFilePath(ITOP_DEFAULT_ENV);
if (file_exists($sConfigFile) && !is_writable($sConfigFile) && $oStep->RequiresWritableConfig()) {
$sRelativePath = utils::GetConfigFilePathRelative(ITOP_DEFAULT_ENV);
$sErrorMsg = "<b>Error:</b> the configuration file '".$sRelativePath."' already exists and cannot be overwritten.";
$sErrorMsg .= "The wizard cannot modify the configuration file for you. If you want to upgrade ".ITOP_APPLICATION.", make sure that the file '<b>".$sRelativePath."</b>' can be modified by the web server.";
throw new Exception($sErrorMsg);
} else {
$oStep->AsyncAction($oPage, $sActionCode, $aParams);
}
$oStep->AsyncAction($oPage, $sActionCode, $aParams);
}
$oPage->output();
break;

View File

@@ -2,9 +2,8 @@ function WizardAsyncAction(sActionCode, oParams, OnErrorFunction)
{
var sStepClass = $('#_class').val();
var sStepState = $('#_state').val();
var sAuthent = $('#authent_token').val();
var oMap = { operation: 'async_action', step_class: sStepClass, step_state: sStepState, code: sActionCode, authent : sAuthent, params: oParams };
var oMap = { operation: 'async_action', step_class: sStepClass, step_state: sStepState, code: sActionCode, params: oParams };
var ErrorFn = OnErrorFunction;
$(document).ajaxError(function(event, request, settings) {

View File

@@ -67,7 +67,34 @@ if (SetupUtils::IsSessionSetupTokenValid()) {
$oWizard->Run();
} else {
SetupUtils::ExitMaintenanceMode(false);
// Force initializing the setup
$oWizard->Start();
$sConfigFile = utils::GetConfigFilePath(ITOP_DEFAULT_ENV);
if (file_exists($sConfigFile)) {
// The configuration file already exists
if (!is_writable($sConfigFile)) {
SetupUtils::ExitReadOnlyMode(false); // Reset readonly mode in case of problem
SetupUtils::EraseSetupToken();
$sRelativePath = utils::GetConfigFilePathRelative(ITOP_DEFAULT_ENV);
$oP = new SetupPage('Installation Cannot Continue');
$oP->add("<h2>Fatal error</h2>\n");
$oP->error("<b>Error:</b> the configuration file '".$sRelativePath."' already exists and cannot be overwritten.");
$oP->p("The wizard cannot modify the configuration file for you. If you want to upgrade ".ITOP_APPLICATION.", make sure that the file '<b>".$sRelativePath."</b>' can be modified by the web server.");
$sButtonsHtml = <<<HTML
<button type="button" class="ibo-button ibo-is-regular ibo-is-primary" onclick="window.location.reload()">Reload</button>
HTML;
$oP->p($sButtonsHtml);
$oP->output();
// Prevent token creation
exit;
} else {
chmod($sConfigFile, 0440);
}
}
SetupUtils::CreateSetupToken();
// Start the setup
$oWizard->Start();
}

View File

@@ -195,30 +195,6 @@ class WizardController
{
SetupLog::Info("=== Setup screen: ".$oStep->GetTitle().' ('.get_class($oStep).')');
$oPage = new SetupPage($oStep->GetTitle());
if ($oStep->RequiresWritableConfig()) {
$sConfigFile = utils::GetConfigFilePath(ITOP_DEFAULT_ENV);
if (file_exists($sConfigFile)) {
// The configuration file already exists
if (!is_writable($sConfigFile)) {
SetupUtils::ExitReadOnlyMode(false); // Reset readonly mode in case of problem
SetupUtils::EraseSetupToken();
$sRelativePath = utils::GetConfigFilePathRelative(ITOP_DEFAULT_ENV);
$oP = new SetupPage('Installation Cannot Continue');
$oP->add("<h2>Fatal error</h2>\n");
$oP->error("<b>Error:</b> the configuration file '".$sRelativePath."' already exists and cannot be overwritten.");
$oP->p("The wizard cannot modify the configuration file for you. If you want to upgrade ".ITOP_APPLICATION.", make sure that the file '<b>".$sRelativePath."</b>' can be modified by the web server.");
$sButtonsHtml = <<<HTML
<button type="button" class="ibo-button ibo-is-regular ibo-is-primary" onclick="window.location.reload()">Reload</button>
HTML;
$oP->p($sButtonsHtml);
$oP->output();
// Prevent token creation
exit;
}
}
}
$oPage->LinkScriptFromAppRoot('setup/setup.js');
$oPage->add('<form id="wiz_form" class="ibo-setup--wizard" method="post">');
$oPage->add('<div class="ibo-setup--wizard--content">');

View File

@@ -21,6 +21,7 @@
/**
* Database Connection parameters screen
*/
use Combodo\iTop\Application\WebPage\WebPage;
class WizStepDBParams extends WizardStep
@@ -76,8 +77,6 @@ class WizStepDBParams extends WizardStep
$sTlsCA,
$sNewDBName
);
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
$oPage->add('</table>');
$sCreateDB = $this->oWizard->GetParameter('create_db', 'yes');
if ($sCreateDB == 'no') {

View File

@@ -18,8 +18,6 @@
* You should have received a copy of the GNU Affero General Public License
*/
use Combodo\iTop\Application\Helper\Session;
require_once(APPROOT.'setup/sequencers/DataAuditSequencer.php');
/**
@@ -77,9 +75,6 @@ class WizStepDataAudit extends WizStepInstall
$sJSONData = json_encode($aInstallParams);
$oPage->add('<input type="hidden" id="installer_parameters" value="'.utils::EscapeHtml($sJSONData).'"/>');
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
if (!$this->CheckDependencies()) {
$oPage->error($this->sDependencyIssue);
$oPage->add_ready_script(<<<JS
@@ -126,12 +121,10 @@ JS);
<input type="hidden" name="$sParamName" value="$sElements"/>
INPUT;
}
$sUID = Session::Get('setup_token');
$oPage->add(
<<<HTML
<form id="data-feature-removal" class="ibo-setup--wizard ibo-is-hidden" method="post" action="$sApplicationUrl">
<input type="hidden" name="operation" value="AnalysisResult"/>
<input type="hidden" name="authent" value="$sUID"/>
$aHiddenInputs
</form>
HTML

View File

@@ -69,13 +69,13 @@ class WizStepDone extends WizardStep
}
$bHasBackup = false;
if (($this->oWizard->GetParameter('mode', '') == 'upgrade') && $this->oWizard->GetParameter('db_backup', false) && $this->oWizard->GetParameter('authent', false)) {
if (($this->oWizard->GetParameter('mode', '') == 'upgrade') && $this->oWizard->GetParameter('db_backup', false)) {
$sBackupDestination = $this->oWizard->GetParameter('db_backup_path', '');
if (file_exists($sBackupDestination.'.tar.gz')) {
$bHasBackup = true;
// To mitigate security risks: pass only the filename without the extension, the download will add the extension itself
$oPage->p('Your backup is ready');
$oPage->p('<a style="background:transparent;" href="'.utils::GetAbsoluteUrlAppRoot(true).'setup/ajax.dataloader.php?operation=async_action&step_class=WizStepDone&params[backup]='.urlencode($sBackupDestination).'&authent='.$this->oWizard->GetParameter('authent', '').'" target="_blank"><img src="../images/icons/icons8-archive-folder.svg" style="border:0;vertical-align:middle;">&nbsp;Download '.basename($sBackupDestination).'</a>');
$oPage->p('<a style="background:transparent;" href="'.utils::GetAbsoluteUrlAppRoot(true).'setup/ajax.dataloader.php?operation=async_action&step_class=WizStepDone&params[backup]='.urlencode($sBackupDestination).'" target="_blank"><img src="../images/icons/icons8-archive-folder.svg" style="border:0;vertical-align:middle;">&nbsp;Download '.basename($sBackupDestination).'</a>');
} else {
$oPage->p('<img src="../images/error.png"/>&nbsp;Warning: Backup creation failed !');
}
@@ -121,8 +121,6 @@ class WizStepDone extends WizardStep
$sTargetEnv = utils::HtmlEntities($this->oWizard->GetParameter('target_env', ITOP_DEFAULT_ENV));
$sForm = '<div class="ibo-setup--wizard--buttons-container" style="text-align:center"><form method="post" class="ibo-setup--enter-itop" action="'.$this->oWizard->GetParameter('application_url').'pages/UI.php?switch_env='.$sTargetEnv.'">';
$sForm .= '<input type="hidden" name="auth_user" value="'.utils::EscapeHtml($this->oWizard->GetParameter('admin_user')).'">';
$sForm .= '<input type="hidden" name="auth_pwd" value="'.utils::EscapeHtml($this->oWizard->GetParameter('admin_pwd')).'">';
$sForm .= "<button id=\"enter_itop\" class=\"ibo-button ibo-is-regular ibo-is-primary\" type=\"submit\">Enter ".ITOP_APPLICATION."</button></div>";
$sForm .= '</form>';
@@ -151,15 +149,6 @@ class WizStepDone extends WizardStep
return 'return false;';
}
/**
* Tells whether this step of the wizard requires that the configuration file be writable
* @return bool True if the wizard will possibly need to modify the configuration at some point
*/
public function RequiresWritableConfig()
{
return false; //This step executes once the config was written and secured
}
public function AsyncAction(WebPage $oPage, $sCode, $aParameters)
{
SetupUtils::EraseSetupToken();

View File

@@ -98,9 +98,6 @@ EOF
$sJSONData = json_encode($aInstallParams);
$oPage->add('<input type="hidden" id="installer_parameters" value="'.utils::EscapeHtml($sJSONData).'"/>');
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
if (!$this->CheckDependencies()) {
$oPage->error($this->sDependencyIssue);
$oPage->add_ready_script(<<<JS

View File

@@ -85,8 +85,6 @@ class WizStepInstallMiscParams extends AbstractWizStepMiscParams
$sChecked = ($sSampleData == 'no') ? 'checked ' : '';
$oPage->p('<input id="sample_data_no" name="sample_data" type="radio" value="no" '.$sChecked.'><label for="sample_data_no">&nbsp;I am installing a <b>production</b> instance, create an empty database to start from.');
$oPage->add('</fieldset>');
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
$oPage->add_ready_script(
<<<EOF
$('#application_url').on('change keyup', function() { WizardUpdateButtons(); } );

View File

@@ -17,6 +17,7 @@
*
* You should have received a copy of the GNU Affero General Public License
*/
use Combodo\iTop\Application\WebPage\WebPage;
/**
@@ -124,9 +125,7 @@ HTML
$sTlsCA
);
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('</div>');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
//$oPage->add('</fieldset>');
$oPage->add_ready_script(
<<<JS

View File

@@ -52,10 +52,6 @@ class WizStepLandingBeforeAudit extends WizStepModulesChoice
*/
public function UpdateWizardStateAndGetNextStep($bMoveForward = true): WizardState
{
// Change the rights to production config file !
$sBuildConfigFile = APPCONF.ITOP_DEFAULT_ENV.'/'.ITOP_CONFIG_FILE;
@chmod($sBuildConfigFile, 0770); // In case it exists: RWX for owner and group, nothing for others
$aWizardSteps = $this->GetWizardSteps();
$this->oWizard->SetWizardSteps($aWizardSteps);
$this->sCurrentState = count($aWizardSteps) - 1;

View File

@@ -238,9 +238,6 @@ class WizStepSummary extends AbstractWizStepInstall
}
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
$oPage->add_ready_script(
<<<JS
$("#db_backup_path").on('change keyup', function() { WizardAsyncAction('check_backup', { db_backup_path: $('#db_backup_path').val() }); });

View File

@@ -65,8 +65,6 @@ class WizStepUpgradeMiscParams extends AbstractWizStepMiscParams
$oPage->add('</table>');
$oPage->add('<span id="graphviz_status"></span>');
$oPage->add('</fieldset>');
$sAuthentToken = $this->oWizard->GetParameter('authent', '');
$oPage->add('<input type="hidden" id="authent_token" value="'.$sAuthentToken.'"/>');
$oPage->add_ready_script(
<<<EOF
$('#application_url').on('change keyup', function() { WizardUpdateButtons(); } );

View File

@@ -46,8 +46,6 @@ class WizStepWelcome extends WizardStep
public function UpdateWizardStateAndGetNextStep($bMoveForward = true): WizardState
{
$sUID = SetupUtils::CreateSetupToken();
$this->oWizard->SetParameter('authent', $sUID);
return new WizardState(WizStepInstallOrUpgrade::class);
}