diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php index 1371127f6..a6aed05f3 100644 --- a/application/cmdbabstract.class.inc.php +++ b/application/cmdbabstract.class.inc.php @@ -4303,7 +4303,7 @@ EOF if (!$bPreview) { // Not in preview mode, do the update for real - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId, false)) { throw new Exception(Dict::S('UI:Error:ObjectAlreadyUpdated')); @@ -4555,7 +4555,9 @@ EOF } $oAppContext = new ApplicationContext(); $oP->add("
\n"); - $oP->add("\n"); + $oP->add("\n"); $oP->add("\n"); $oP->add("\n"); $oP->add($oAppContext->GetForForm()); diff --git a/application/portalwebpage.class.inc.php b/application/portalwebpage.class.inc.php index 7af335229..1f9e57808 100644 --- a/application/portalwebpage.class.inc.php +++ b/application/portalwebpage.class.inc.php @@ -807,7 +807,7 @@ EOF */ public function DoUpdateObjectFromPostedForm(DBObject $oObj, $aAttList = null) { - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId)) { throw new TransactionException(); diff --git a/application/utils.inc.php b/application/utils.inc.php index c708e0a7c..f02e6a60d 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -314,10 +314,20 @@ class utils { switch($sSanitizationFilter) { - case 'parameter': - $retValue = filter_var($value, FILTER_VALIDATE_REGEXP, array("options"=>array("regexp"=>'/^([ A-Za-z0-9_=-]|%3D|%2B|%2F)*$/'))); // the '=', '%3D, '%2B', '%2F' characters are used in serialized filters (starting 2.5, only the url encoded versions are presents, but the "=" is kept for BC) + case 'transaction_id': + // same as parameter type but keep the dot character + // see N°1835 : when using file transaction_id on Windows you get *.tmp tokens + // it must be included at the regexp beginning otherwise you'll get an invalid character error + $retValue = filter_var($value, FILTER_VALIDATE_REGEXP, + array("options" => array("regexp" => '/^[\. A-Za-z0-9_=-]*$/'))); break; - + + case 'parameter': + $retValue = filter_var($value, FILTER_VALIDATE_REGEXP, + array("options" => array("regexp" => '/^[ A-Za-z0-9_=-]*$/'))); // the '=', '%3D, '%2B', '%2F' + // characters are used in serialized filters (starting 2.5, only the url encoded versions are presents, but the "=" is kept for BC) + break; + case 'field_name': $retValue = filter_var($value, FILTER_VALIDATE_REGEXP, array("options"=>array("regexp"=>'/^[A-Za-z0-9_]+(->[A-Za-z0-9_]+)*$/'))); // att_code or att_code->name or AttCode->Name or AttCode->Key2->Name break; diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 3e5464e6a..bb303c5a5 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -993,7 +993,7 @@ class Config 'transaction_storage' => array( 'type' => 'string', 'description' => 'The type of mechanism to use for storing the unique identifiers for transactions (Session|File).', - 'default' => 'Session', + 'default' => 'File', 'value' => '', 'source_of_value' => '', 'show_in_conf_sample' => false, @@ -1150,6 +1150,14 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ), + 'regenerate_session_id_enabled' => array( + 'type' => 'bool', + 'description' => 'If true then session id will be regenerated on each login, to prevent session fixation.', + 'default' => true, + 'value' => true, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ), ); public function IsProperty($sPropCode) diff --git a/core/inlineimage.class.inc.php b/core/inlineimage.class.inc.php index cbeb60353..159ddb21e 100644 --- a/core/inlineimage.class.inc.php +++ b/core/inlineimage.class.inc.php @@ -161,7 +161,7 @@ class InlineImage extends DBObject */ public static function FinalizeInlineImages(DBObject $oObject) { - $iTransactionId = utils::ReadParam('transaction_id', null); + $iTransactionId = utils::ReadParam('transaction_id', null, false, 'transaction_id'); if (!is_null($iTransactionId)) { // Attach new (temporary) inline images diff --git a/core/userrights.class.inc.php b/core/userrights.class.inc.php index d33ef5025..5c746bbb1 100644 --- a/core/userrights.class.inc.php +++ b/core/userrights.class.inc.php @@ -1331,15 +1331,24 @@ class UserRights { $_SESSION['profile_list'] = self::ListProfiles(); } - // Protection against session fixation/injection: generate a new session id. - - // Alas a PHP bug (technically a bug in the memcache session handler, https://bugs.php.net/bug.php?id=71187) - // causes session_regenerate_id to fail with a catchable fatal error in PHP 7.0 if the session handler is memcache(d). - // The bug has been fixed in PHP 7.2, but in case session_regenerate_id() - // fails we just silently ignore the error and keep the same session id... - $old_error_handler = set_error_handler(array(__CLASS__, 'VoidErrorHandler')); - session_regenerate_id(); - if ($old_error_handler !== null) set_error_handler($old_error_handler); + + $oConfig = MetaModel::GetConfig(); + $bSessionIdRegeneration = $oConfig->Get('regenerate_session_id_enabled'); + if ($bSessionIdRegeneration) + { + // Protection against session fixation/injection: generate a new session id. + + // Alas a PHP bug (technically a bug in the memcache session handler, https://bugs.php.net/bug.php?id=71187) + // causes session_regenerate_id to fail with a catchable fatal error in PHP 7.0 if the session handler is memcache(d). + // The bug has been fixed in PHP 7.2, but in case session_regenerate_id() + // fails we just silently ignore the error and keep the same session id... + $old_error_handler = set_error_handler(array(__CLASS__, 'VoidErrorHandler')); + session_regenerate_id(); + if ($old_error_handler !== null) + { + set_error_handler($old_error_handler); + } + } } public static function _ResetSessionCache() diff --git a/datamodels/2.x/itop-attachments/main.attachments.php b/datamodels/2.x/itop-attachments/main.attachments.php index 6e645654e..358a4c8e2 100755 --- a/datamodels/2.x/itop-attachments/main.attachments.php +++ b/datamodels/2.x/itop-attachments/main.attachments.php @@ -473,7 +473,7 @@ EOF // Leave silently if there is no trace of the attachment form return; } - $sTransactionId = utils::ReadParam('transaction_id', null); + $sTransactionId = utils::ReadParam('transaction_id', null, false, 'transaction_id'); if (!is_null($sTransactionId)) { $aActions = array(); diff --git a/datamodels/2.x/itop-config/config.php b/datamodels/2.x/itop-config/config.php index f52ade8f7..e91412c63 100644 --- a/datamodels/2.x/itop-config/config.php +++ b/datamodels/2.x/itop-config/config.php @@ -126,7 +126,7 @@ try } if ($sOperation == 'save') { - $sTransactionId = utils::ReadParam('transaction_id', ''); + $sTransactionId = utils::ReadParam('transaction_id', '', false, 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId, true)) { $oP->add("
Error: invalid Transaction ID. The configuration was NOT modified.
"); diff --git a/pages/UI.php b/pages/UI.php index 8a70fb3ac..dea8a5acc 100644 --- a/pages/UI.php +++ b/pages/UI.php @@ -885,7 +885,7 @@ EOF $sClass = utils::ReadPostedParam('class', ''); $sClassLabel = MetaModel::GetName($sClass); $id = utils::ReadPostedParam('id', ''); - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); if ( empty($sClass) || empty($id)) // TO DO: check that the class name is valid ! { throw new ApplicationException(Dict::Format('UI:Error:2ParametersMissing', 'class', 'id')); @@ -1006,7 +1006,7 @@ EOF case 'bulk_delete_confirmed': // Confirm bulk deletion of objects $oP->DisableBreadCrumb(); - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId)) { throw new ApplicationException(Dict::S('UI:Error:ObjectsAlreadyDeleted')); @@ -1074,7 +1074,7 @@ EOF $oP->DisableBreadCrumb(); $sClass = utils::ReadPostedParam('class', '', 'class'); $sClassLabel = MetaModel::GetName($sClass); - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); $aErrors = array(); if ( empty($sClass) ) // TO DO: check that the class name is valid ! { @@ -1358,7 +1358,7 @@ EOF { throw new ApplicationException(Dict::Format('UI:Error:3ParametersMissing', 'filter', 'stimulus', 'state')); } - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId)) { $oP->p(Dict::S('UI:Error:ObjectAlreadyUpdated')); @@ -1510,7 +1510,7 @@ EOF $oP->DisableBreadCrumb(); $sClass = utils::ReadPostedParam('class', ''); $id = utils::ReadPostedParam('id', ''); - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); $sStimulus = utils::ReadPostedParam('stimulus', ''); if ( empty($sClass) || empty($id) || empty($sStimulus) ) // TO DO: check that the class name is valid ! { diff --git a/pages/ajax.render.php b/pages/ajax.render.php index 4299cb768..781966bf0 100644 --- a/pages/ajax.render.php +++ b/pages/ajax.render.php @@ -709,7 +709,7 @@ try $oObj = $oWizardHelper->GetTargetObject(); $sClass = $oWizardHelper->GetTargetClass(); $sTargetState = utils::ReadParam('target_state', ''); - $iTransactionId = utils::ReadParam('transaction_id', ''); + $iTransactionId = utils::ReadParam('transaction_id', '', false, 'transaction_id'); $oObj->Set(MetaModel::GetStateAttributeCode($sClass), $sTargetState); cmdbAbstractObject::DisplayCreationForm($oPage, $sClass, $oObj, array(), array('action' => utils::GetAbsoluteUrlAppRoot().'pages/UI.php', 'transaction_id' => $iTransactionId)); break; @@ -903,7 +903,7 @@ try case 'on_form_cancel': // Called when a creation/modification form is cancelled by the end-user // Let's take this opportunity to inform the plug-ins so that they can perform some cleanup - $iTransactionId = utils::ReadParam('transaction_id', 0); + $iTransactionId = utils::ReadParam('transaction_id', 0, false, 'transaction_id'); $sTempId = session_id().'_'.$iTransactionId; InlineImage::OnFormCancel($sTempId); foreach(MetaModel::EnumPlugins('iApplicationUIExtension') as $oExtensionInstance) @@ -942,7 +942,7 @@ try break; case 'import_dashboard': - $sTransactionId = utils::ReadParam('transaction_id', '', false, 'raw_data'); + $sTransactionId = utils::ReadParam('transaction_id', '', false, 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId, true)) { throw new SecurityException('ajax.render.php import_dashboard : invalid transaction_id'); diff --git a/portal/index.php b/portal/index.php index e246ab6d0..f2bada8e3 100644 --- a/portal/index.php +++ b/portal/index.php @@ -619,7 +619,7 @@ EOF function DoCreateRequest($oP, $oUserOrg) { $aParameters = $oP->ReadAllParams(PORTAL_ALL_PARAMS.',template_id'); - $sTransactionId = utils::ReadPostedParam('transaction_id', ''); + $sTransactionId = utils::ReadPostedParam('transaction_id', '', 'transaction_id'); if (!utils::IsTransactionValid($sTransactionId)) { $oP->add("

".Dict::S('UI:Error:ObjectAlreadyCreated')."

\n");