(retrofit from trunk) Security: prevent grouping on password fields since it may lead to disclosure of the encrypted version of the password.

SVN:2.1.1[4246]

application/applicationextension.inc.php

@@ -783,6 +783,10 @@ class RestUtils
 		{
 			$realValue = self::MakeValue($sClass, $sAttCode, $value);
 			$oSearch->AddCondition($sAttCode, $realValue, '=');
+			if (is_object($value) || is_array($value))
+			{
+				$value = json_encode($value);
+			}
 			$aCriteriaReport[] = "$sAttCode: $value ($realValue)";
 		}
 		$oSet = new DBObjectSet($oSearch);

application/dashboard.class.inc.php

@@ -312,7 +312,7 @@ abstract class Dashboard
 	
 	public function Render($oPage, $bEditMode = false, $aExtraParams = array())
 	{
-		$oPage->add('<h1>'.Dict::S($this->sTitle).'</h1>');
+		$oPage->add('<h1>'.htmlentities(Dict::S($this->sTitle), ENT_QUOTES, 'UTF-8', false).'</h1>');
 		$oLayout = new $this->sLayoutClass;
 		$oLayout->Render($oPage, $this->aCells, $bEditMode, $aExtraParams);
 		if (!$bEditMode)

application/dashlet.class.inc.php

@@ -733,7 +733,8 @@ abstract class DashletGroupBy extends Dashlet
 			if (is_subclass_of($sAttType, 'AttributeFriendlyName')) continue;
 			if ($sAttType == 'AttributeExternalField') continue;
 			if (is_subclass_of($sAttType, 'AttributeExternalField')) continue;
-
+			if ($sAttType == 'AttributeOneWayPassword') continue;
+			
 			$sLabel = $this->oModelReflection->GetLabel($sClass, $sAttCode);
 			$aGroupBy[$sAttCode] = $sLabel;
 

application/displayblock.class.inc.php

@@ -393,7 +393,7 @@ class DisplayBlock
 			{
 				if (isset($aExtraParams['group_by_label']))
 				{
-					$oGroupByExp = Expression::FromOQL($aExtraParams['group_by']);
+					$oGroupByExp = Expression::FromOQL($aExtraParams['group_by']);					
 					$sGroupByLabel = $aExtraParams['group_by_label'];
 				}
 				else
@@ -404,6 +404,21 @@ class DisplayBlock
 					$sGroupByLabel = MetaModel::GetLabel($this->m_oFilter->GetClass(), $aExtraParams['group_by']);
 				}
 
+				// Security filtering
+				$aFields = $oGroupByExp->ListRequiredFields();
+				foreach($aFields as $sFieldAlias)
+				{
+					if (preg_match('/^([^.]+)\\.([^.]+)$/', $sFieldAlias, $aMatches))
+					{
+						$sFieldClass = $this->m_oFilter->GetClassName($aMatches[1]);
+						$oAttDef = MetaModel::GetAttributeDef($sFieldClass, $aMatches[2]);
+						if ($oAttDef instanceof AttributeOneWayPassword)
+						{
+							throw new Exception('Grouping on password fields is not supported.');
+						}
+					}
+				}
+				
 				$aGroupBy = array();
 				$aGroupBy['grouped_by_1'] = $oGroupByExp;
 				$sSql = MetaModel::MakeGroupByQuery($this->m_oFilter, $aQueryParams, $aGroupBy, true);

application/loginwebpage.class.inc.php

@@ -305,16 +305,20 @@ class LoginWebPage extends NiceWebPage
 		{
 			$this->add("<p>".Dict::Format('UI:ResetPwd-Error-WrongLogin', $sAuthUser)."</p>\n");
 		}
-		elseif ($oUser->Get('reset_pwd_token') != $sToken)
-		{
-			$this->add("<p>".Dict::S('UI:ResetPwd-Error-InvalidToken')."</p>\n");
-		}
 		else
 		{
-			$this->add("<p>".Dict::Format('UI:ResetPwd-Error-EnterPassword', $oUser->GetFriendlyName())."</p>\n");
-
-			$sInconsistenPwdMsg = Dict::S('UI:Login:RetypePwdDoesNotMatch');
-			$this->add_script(
+			$oEncryptedToken = $oUser->Get('reset_pwd_token');
+			
+			if (!$oEncryptedToken->CheckPassword($sToken))
+			{
+				$this->add("<p>".Dict::S('UI:ResetPwd-Error-InvalidToken')."</p>\n");
+			}
+			else
+			{
+				$this->add("<p>".Dict::Format('UI:ResetPwd-Error-EnterPassword', $oUser->GetFriendlyName())."</p>\n");
+	
+				$sInconsistenPwdMsg = Dict::S('UI:Login:RetypePwdDoesNotMatch');
+				$this->add_script(
 <<<EOF
 function DoCheckPwd()
 {
@@ -326,18 +330,19 @@ function DoCheckPwd()
 	return true;
 }
 EOF
-			);
-			$this->add("<form method=\"post\">\n");
-			$this->add("<table>\n");
-			$this->add("<tr><td style=\"text-align:right\"><label for=\"new_pwd\">".Dict::S('UI:Login:NewPasswordPrompt').":</label></td><td style=\"text-align:left\"><input type=\"password\" id=\"new_pwd\" name=\"new_pwd\" value=\"\" /></td></tr>\n");
-			$this->add("<tr><td style=\"text-align:right\"><label for=\"retype_new_pwd\">".Dict::S('UI:Login:RetypeNewPasswordPrompt').":</label></td><td style=\"text-align:left\"><input type=\"password\" id=\"retype_new_pwd\" name=\"retype_new_pwd\" value=\"\" /></td></tr>\n");
-			$this->add("<tr><td colspan=\"2\" class=\"center v-spacer\"><span class=\"btn_border\"><input type=\"submit\" onClick=\"return DoCheckPwd();\" value=\"".Dict::S('UI:Button:ChangePassword')."\" /></span></td></tr>\n");
-			$this->add("</table>\n");
-			$this->add("<input type=\"hidden\" name=\"loginop\" value=\"do_reset_pwd\" />\n");
-			$this->add("<input type=\"hidden\" name=\"auth_user\" value=\"".htmlentities($sAuthUser, ENT_QUOTES, 'UTF-8')."\" />\n");
-			$this->add("<input type=\"hidden\" name=\"token\" value=\"".htmlentities($sToken, ENT_QUOTES, 'UTF-8')."\" />\n");
-			$this->add("</form>\n");
-			$this->add("</div\n");
+				);
+				$this->add("<form method=\"post\">\n");
+				$this->add("<table>\n");
+				$this->add("<tr><td style=\"text-align:right\"><label for=\"new_pwd\">".Dict::S('UI:Login:NewPasswordPrompt').":</label></td><td style=\"text-align:left\"><input type=\"password\" id=\"new_pwd\" name=\"new_pwd\" value=\"\" /></td></tr>\n");
+				$this->add("<tr><td style=\"text-align:right\"><label for=\"retype_new_pwd\">".Dict::S('UI:Login:RetypeNewPasswordPrompt').":</label></td><td style=\"text-align:left\"><input type=\"password\" id=\"retype_new_pwd\" name=\"retype_new_pwd\" value=\"\" /></td></tr>\n");
+				$this->add("<tr><td colspan=\"2\" class=\"center v-spacer\"><span class=\"btn_border\"><input type=\"submit\" onClick=\"return DoCheckPwd();\" value=\"".Dict::S('UI:Button:ChangePassword')."\" /></span></td></tr>\n");
+				$this->add("</table>\n");
+				$this->add("<input type=\"hidden\" name=\"loginop\" value=\"do_reset_pwd\" />\n");
+				$this->add("<input type=\"hidden\" name=\"auth_user\" value=\"".htmlentities($sAuthUser, ENT_QUOTES, 'UTF-8')."\" />\n");
+				$this->add("<input type=\"hidden\" name=\"token\" value=\"".htmlentities($sToken, ENT_QUOTES, 'UTF-8')."\" />\n");
+				$this->add("</form>\n");
+				$this->add("</div\n");
+			}
 		}
 	}
 
@@ -357,21 +362,25 @@ EOF
 		{
 			$this->add("<p>".Dict::Format('UI:ResetPwd-Error-WrongLogin', $sAuthUser)."</p>\n");
 		}
-		elseif ($oUser->Get('reset_pwd_token') != $sToken)
-		{
-			$this->add("<p>".Dict::S('UI:ResetPwd-Error-InvalidToken')."</p>\n");
-		}
 		else
 		{
-			// Trash the token and change the password
-			$oUser->Set('reset_pwd_token', '');
-			$oUser->SetPassword($sNewPwd); // Does record the change into the DB
-
-			$this->add("<p>".Dict::S('UI:ResetPwd-Ready')."</p>");
-			$sUrl = utils::GetAbsoluteUrlAppRoot();
-			$this->add("<p><a href=\"$sUrl\">".Dict::S('UI:ResetPwd-Login')."</a></p>");
+			$oEncryptedPassword = $oUser->Get('reset_pwd_token');
+			if (!$oEncryptedPassword->CheckPassword($sToken))
+			{
+				$this->add("<p>".Dict::S('UI:ResetPwd-Error-InvalidToken')."</p>\n");
+			}
+			else
+			{
+				// Trash the token and change the password
+				$oUser->Set('reset_pwd_token', '');
+				$oUser->SetPassword($sNewPwd); // Does record the change into the DB
+	
+				$this->add("<p>".Dict::S('UI:ResetPwd-Ready')."</p>");
+				$sUrl = utils::GetAbsoluteUrlAppRoot();
+				$this->add("<p><a href=\"$sUrl\">".Dict::S('UI:ResetPwd-Login')."</a></p>");
+			}
+			$this->add("</div\n");
 		}
-		$this->add("</div\n");
 	}
 
 	public function DisplayChangePwdForm($bFailedLogin = false)

application/startup.inc.php

@@ -28,10 +28,11 @@ require_once(APPROOT.'/core/cmdbobject.class.inc.php');
 require_once(APPROOT.'/application/utils.inc.php');
 session_name('itop-'.md5(APPROOT));
 session_start();
-if (isset($_REQUEST['switch_env']))
+$sSwitchEnv = utils::ReadParam('switch_env', null);
+if (($sSwitchEnv != null) && (file_exists(APPCONF.$sSwitchEnv.'/'.ITOP_CONFIG_FILE)))
 {
-	$sEnv = $_REQUEST['switch_env'];
-	$_SESSION['itop_env'] = $sEnv;
+	$_SESSION['itop_env'] = $sSwitchEnv;
+	$sEnv = $sSwitchEnv;
 	// TODO: reset the credentials as well ??
 }
 else if (isset($_SESSION['itop_env']))

application/transaction.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2012 Combodo SARL
+// Copyright (C) 2010-2015 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -19,7 +19,8 @@
 /**
  * This class records the pending "transactions" corresponding to forms that have not been
  * submitted yet, in order to prevent double submissions. When created a transaction remains valid
- * until the user's session expires
+ * until the user's session expires. This class is actually a wrapper to the underlying implementation
+ * which choice is configured via the parameter 'transaction_storage'
  *  
  * @package     iTop
  * @copyright   Copyright (C) 2010-2012 Combodo SARL
@@ -28,6 +29,81 @@
 
 
 class privUITransaction
+{
+	/**
+	 * Create a new transaction id, store it in the session and return its id
+	 * @param void
+	 * @return int The identifier of the new transaction
+	 */
+	public static function GetNewTransactionId()
+	{
+		$bTransactionsEnabled = MetaModel::GetConfig()->Get('transactions_enabled');
+		if (!$bTransactionsEnabled)
+		{
+			return 'notransactions'; // Any value will do
+		}
+		$sClass = 'privUITransaction'.MetaModel::GetConfig()->Get('transaction_storage');
+		if (!class_exists($sClass, false))
+		{
+			IssueLog::Error("Incorrect value '".MetaModel::GetConfig()->Get('transaction_storage')."' for 'transaction_storage', the class '$sClass' does not exists. Using privUITransactionSession instead for storing sessions.");
+			$sClass = 'privUITransactionSession';
+		}
+
+		return (string)$sClass::GetNewTransactionId();
+	}
+
+	/**
+	 * Check whether a transaction is valid or not and (optionally) remove the valid transaction from
+	 * the session so that another call to IsTransactionValid for the same transaction id
+	 * will return false
+	 * @param int $id Identifier of the transaction, as returned by GetNewTransactionId
+	 * @param bool $bRemoveTransaction True if the transaction must be removed
+	 * @return bool True if the transaction is valid, false otherwise
+	 */
+	public static function IsTransactionValid($id, $bRemoveTransaction = true)
+	{
+		$bTransactionsEnabled = MetaModel::GetConfig()->Get('transactions_enabled');
+		if (!$bTransactionsEnabled)
+		{
+			return true; // All values are valid
+		}
+		$sClass = 'privUITransaction'.MetaModel::GetConfig()->Get('transaction_storage');
+		if (!class_exists($sClass, false))
+		{
+			$sClass = 'privUITransactionSession';
+		}
+		
+		return $sClass::IsTransactionValid($id, $bRemoveTransaction);
+	}
+
+	/**
+	 * Removes the transaction specified by its id
+	 * @param int $id The Identifier (as returned by GetNewTranscationId) of the transaction to be removed.
+	 * @return void
+	 */
+	public static function RemoveTransaction($id)
+	{
+		$bTransactionsEnabled = MetaModel::GetConfig()->Get('transactions_enabled');
+		if (!$bTransactionsEnabled)
+		{
+			return; // Nothing to do
+		}
+		$sClass = 'privUITransaction'.MetaModel::GetConfig()->Get('transaction_storage');
+		if (!class_exists($sClass, false))
+		{
+			$sClass = 'privUITransactionSession';
+		}
+		
+		$sClass::RemoveTransaction($id);
+	}
+}
+
+/**
+ * The original (and by default) mechanism for storing transaction information
+ * as an array in the $_SESSION variable
+ *
+ */
+class privUITransactionSession
 {
 	/**
 	 * Create a new transaction id, store it in the session and return its id
@@ -99,4 +175,178 @@ class privUITransaction
 		}		
 	}
 }
-?>
+
+/**
+ * An alternate implementation for storing the transactions as temporary files
+ * Useful when using an in-memory storage for the session which do not
+ * guarantee mutual exclusion for writing
+ */
+class privUITransactionFile
+{
+	/**
+	 * Create a new transaction id, store it in the session and return its id
+	 * @param void
+	 * @return int The identifier of the new transaction
+	 */
+	public static function GetNewTransactionId()
+	{
+		if (!is_dir(APPROOT.'data/transactions'))
+		{
+			if (!is_writable(APPROOT.'data'))
+			{
+				throw new Exception('The directory "'.APPROOT.'data" must be writable to the application.');
+			}
+			if (!@mkdir(APPROOT.'data/transactions'))
+			{
+				throw new Exception('Failed to create the directory "'.APPROOT.'data/transactions". Ajust the rights on the parent directory or let an administrator create the transactions directory and give the web sever enough rights to write into it.');
+			}
+		}
+		if (!is_writable(APPROOT.'data/transactions'))
+		{
+			throw new Exception('The directory "'.APPROOT.'data/transactions" must be writable to the application.');
+		}
+		self::CleanupOldTransactions();
+		$id = basename(tempnam(APPROOT.'data/transactions', self::GetUserPrefix()));
+		self::Info('GetNewTransactionId: Created transaction: '.$id);
+
+		return (string)$id;
+	}
+
+	/**
+	 * Check whether a transaction is valid or not and (optionally) remove the valid transaction from
+	 * the session so that another call to IsTransactionValid for the same transaction id
+	 * will return false
+	 * @param int $id Identifier of the transaction, as returned by GetNewTransactionId
+	 * @param bool $bRemoveTransaction True if the transaction must be removed
+	 * @return bool True if the transaction is valid, false otherwise
+	 */
+	public static function IsTransactionValid($id, $bRemoveTransaction = true)
+	{
+		$sFilepath = APPROOT.'data/transactions/'.$id;
+		clearstatcache(true, $sFilepath);
+		$bResult = file_exists($sFilepath);
+		if ($bResult)
+		{
+			if ($bRemoveTransaction)
+			{
+				$bResult = @unlink($sFilepath);
+				if (!$bResult)
+				{
+					self::Error('IsTransactionValid: FAILED to remove transaction '.$id);
+				}
+				else
+				{
+					self::Info('IsTransactionValid: OK. Removed transaction: '.$id);
+				}
+			}
+		}
+		else
+		{
+			self::Info("IsTransactionValid: Transaction '$id' not found. Pending transactions for this user:\n".implode("\n", self::GetPendingTransactions()));
+		}
+		return $bResult;
+	}
+
+	/**
+	 * Removes the transaction specified by its id
+	 * @param int $id The Identifier (as returned by GetNewTransactionId) of the transaction to be removed.
+	 * @return void
+	 */
+	public static function RemoveTransaction($id)
+	{
+		$bSuccess = true;
+		$sFilepath = APPROOT.'data/transactions/'.$id;
+		clearstatcache(true, $sFilepath);
+		if(!file_exists($sFilepath))
+		{
+			$bSuccess = false;
+			self::Error("RemoveTransaction: Transaction '$id' not found. Pending transactions for this user:\n".implode("\n", self::GetPendingTransactions()));
+		}
+		$bSuccess = @unlink($sFilepath);
+		if (!$bSuccess)
+		{
+			self::Error('RemoveTransaction: FAILED to remove transaction '.$id);
+		}
+		else
+		{
+			self::Info('RemoveTransaction: OK '.$id);
+		}
+		return $bSuccess;
+	}
+
+	/**
+	 * Cleanup old transactions which have been pending since more than 24 hours
+	 * Use filemtime instead of filectime since filectime may be affected by operations on the directory (like changing the access rights)
+	 */
+	protected static function CleanupOldTransactions()
+	{
+		$iLimit = time() - 24*3600;
+		clearstatcache();
+		$aTransactions = glob(APPROOT.'data/transactions/*-*');
+		foreach($aTransactions as $sFileName)
+		{
+			if (filemtime($sFileName) < $iLimit)
+			{
+				@unlink($sFileName);
+				self::Info('CleanupOldTransactions: Deleted transaction: '.$sFileName);
+			}
+		}
+	}
+
+	/**
+	 * For debugging purposes: gets the pending transactions of the current user
+	 * as an array, with the date of the creation of the transaction file
+	 */
+	protected static function GetPendingTransactions()
+	{
+		clearstatcache();
+		$aResult = array();
+		$aTransactions = glob(APPROOT.'data/transactions/'.self::GetUserPrefix().'*');
+		foreach($aTransactions as $sFileName)
+		{
+			$aResult[] = date('Y-m-d H:i:s', filemtime($sFileName)).' - '.basename($sFileName);
+		}
+		sort($aResult);
+		return $aResult;
+	}
+
+	protected static function GetUserPrefix()
+	{
+		$sPrefix = substr(UserRights::GetUser(), 0, 10);
+		$sPrefix = preg_replace('/[^a-zA-Z0-9-_]/', '_', $sPrefix);
+		return $sPrefix.'-';
+	}
+
+	protected static function Info($sText)
+	{
+		self::Write('Info | '.$sText);
+	}
+
+	protected static function Warning($sText)
+	{
+		self::Write('Warning | '.$sText);
+	}
+			
+	protected static function Error($sText)
+	{
+		self::Write('Error | '.$sText);
+	}
+	
+	protected static function Write($sText)
+	{
+		$bLogEnabled = MetaModel::GetConfig()->Get('log_transactions');
+		if ($bLogEnabled)
+		{
+		$hLogFile = @fopen(APPROOT.'log/transactions.log', 'a');
+		if ($hLogFile !== false)
+		{
+			flock($hLogFile, LOCK_EX);
+			$sDate = date('Y-m-d H:i:s');
+			fwrite($hLogFile, "$sDate | $sText\n");
+			fflush($hLogFile);
+			flock($hLogFile, LOCK_UN);
+			fclose($hLogFile);
+			}
+		}
+	}
+}

application/ui.linksdirectwidget.class.inc.php

@@ -294,7 +294,7 @@ class UILinksWidgetDirect
 		$valuesDef = $oLinksetDef->GetValuesDef();				
 		if ($valuesDef === null)
 		{
-			$oFilter = new DBObjectSearch($this->sLinkedClass);
+			$oFilter = new DBObjectSearch($sRemoteClass);
 		}
 		else
 		{

core/config.class.inc.php

@@ -809,6 +809,54 @@ class Config
 			'source_of_value' => '',
 			'show_in_conf_sample' => false,
 		),
+		'transaction_storage' => array(
+			'type' => 'string',
+			'description' => 'The type of mechanism to use for storing the unique identifiers for transactions (Session|File).',
+			'default' => 'Session',
+			'value' => '',
+			'source_of_value' => '',
+			'show_in_conf_sample' => false,
+		),
+		'transactions_enabled' => array(
+			'type' => 'bool',
+			'description' => 'Whether or not the whole mechanism to prevent multiple submissions of a page is enabled.',
+			'default' => true,
+			'value' => '',
+			'source_of_value' => '',
+			'show_in_conf_sample' => false,
+		),
+		'log_transactions' => array(
+			'type' => 'bool',
+			'description' => 'Whether or not to enable the debug log for the transactions.',
+			'default' => false,
+			'value' => '',
+			'source_of_value' => '',
+			'show_in_conf_sample' => false,
+		),
+		'concurrent_lock_enabled' => array(
+			'type' => 'bool',
+			'description' => 'Whether or not to activate the locking mechanism in order to prevent concurrent edition of the same object.',
+			'default' => true,
+			'value' => '',
+			'source_of_value' => '',
+			'show_in_conf_sample' => true,
+		),
+		'concurrent_lock_expiration_delay' => array(
+			'type' => 'integer',
+			'description' => 'Delay (in seconds) for a concurrent lock to expire',
+			'default' => 120,
+			'value' => '',
+			'source_of_value' => '',
+			'show_in_conf_sample' => false,
+		), 
+		'concurrent_lock_override_profiles' => array(
+			'type' => 'array',
+			'description' => 'The list of profiles allowed to "kill" a lock',
+			'default' => array('Administrator'),
+			'value' => '',
+			'source_of_value' => '',
+			'show_in_conf_sample' => false,
+		), 
 	);
 
 	public function IsProperty($sPropCode)
@@ -1462,6 +1510,8 @@ class Config
 		$aSettings['log_notification'] = $this->m_bLogNotification;
 		$aSettings['log_issue'] = $this->m_bLogIssue;
 		$aSettings['log_web_service'] = $this->m_bLogWebService;
+		$aSettings['log_queries'] = $this->m_bLogQueries;
+		$aSettings['query_cache_enabled'] = $this->m_bQueryCacheEnabled;
 		$aSettings['min_display_limit'] = $this->m_iMinDisplayLimit;
 		$aSettings['max_display_limit'] = $this->m_iMaxDisplayLimit;
 		$aSettings['standard_reload_interval'] = $this->m_iStandardReloadInterval;
@@ -1469,6 +1519,7 @@ class Config
 		$aSettings['secure_connection_required'] = $this->m_bSecureConnectionRequired;
 		$aSettings['default_language'] = $this->m_sDefaultLanguage;
 		$aSettings['allowed_login_types'] = $this->m_sAllowedLoginTypes;
+		$aSettings['ext_auth_variable'] = $this->m_sExtAuthVariable;
 		$aSettings['encryption_key'] = $this->m_sEncryptionKey;
 		$aSettings['csv_import_charsets'] = $this->m_aCharsets;
 
@@ -1534,6 +1585,8 @@ class Config
 				'log_notification' => $this->m_bLogNotification,
 				'log_issue' => $this->m_bLogIssue,
 				'log_web_service' => $this->m_bLogWebService,
+				'log_queries' => $this->m_bLogQueries,
+				'query_cache_enabled' => $this->m_bQueryCacheEnabled,
 				'secure_connection_required' => $this->m_bSecureConnectionRequired,
 			);
 			foreach($aBoolValues as $sKey => $bValue)
@@ -1572,6 +1625,7 @@ class Config
 				'db_collation' => $this->m_sDBCollation,
 				'default_language' => $this->m_sDefaultLanguage,
 				'allowed_login_types' => $this->m_sAllowedLoginTypes,
+				'ext_auth_variable' => $this->m_sExtAuthVariable,
 				'encryption_key' => $this->m_sEncryptionKey,
 				'csv_import_charsets' => $this->m_aCharsets,
 			);

core/dbobjectsearch.class.php

@@ -53,9 +53,8 @@ class DBObjectSearch
 	public function __construct($sClass, $sClassAlias = null)
 	{
 		if (is_null($sClassAlias)) $sClassAlias = $sClass;
-		assert('is_string($sClass)');
-		assert('MetaModel::IsValidClass($sClass)'); // #@# could do better than an assert, or at least give the caller's reference
-		// => idee d'un assert avec call stack (autre utilisation = echec sur query SQL)
+		if(!is_string($sClass)) throw new Exception('DBObjectSearch::__construct called with a non-string parameter: $sClass = '.print_r($sClass, true));
+		if(!MetaModel::IsValidClass($sClass)) throw new Exception('DBObjectSearch::__construct called for an invalid class: "'.$sClass.'"');
 
 		$this->m_aSelectedClasses = array($sClassAlias => $sClass);
 		$this->m_aClasses = array($sClassAlias => $sClass);
@@ -449,7 +448,7 @@ class DBObjectSearch
 
 	public function AddCondition($sFilterCode, $value, $sOpCode = null)
 	{
-		MyHelpers::CheckKeyInArray('filter code', $sFilterCode, MetaModel::GetClassFilterDefs($this->GetClass()));
+		MyHelpers::CheckKeyInArray('filter code in class: '.$this->GetClass(), $sFilterCode, MetaModel::GetClassFilterDefs($this->GetClass()));
 		$oFilterDef = MetaModel::GetClassFilterDef($this->GetClass(), $sFilterCode);
 
 		$oField = new FieldExpression($sFilterCode, $this->GetClassAlias());

core/log.class.inc.php

@@ -59,8 +59,11 @@ class FileLog
 		$hLogFile = @fopen($this->m_sFile, 'a');
 		if ($hLogFile !== false)
 		{
+			flock($hLogFile, LOCK_EX);
 			$sDate = date('Y-m-d H:i:s');
 			fwrite($hLogFile, "$sDate | $sText\n");
+			fflush($hLogFile);
+			flock($hLogFile, LOCK_UN);
 			fclose($hLogFile);
 		}
 	}

core/metamodel.class.php

@@ -3079,7 +3079,7 @@ abstract class MetaModel
 									$sExtAttCode = $oAtt->GetExtAttCode();
 									// Translate mainclass.extfield => remoteclassalias.remotefieldcode
 									$oRemoteAttDef = self::GetAttributeDef($sKeyClass, $sExtAttCode);
-									foreach ($oRemoteAttDef->GetSQLExpressions() as $sColID => $sRemoteAttExpr)
+									foreach ($oRemoteAttDef->GetSQLExpressions() as $sColId => $sRemoteAttExpr)
 									{
 										$aTranslateNow[$sTargetAlias][$sAttCode.$sColId] = new FieldExpression($sExtAttCode, $sKeyClassAlias);
 //echo "<p><b>aTranslateNow[$sTargetAlias][$sAttCode.$sColId] = new FieldExpression($sExtAttCode, $sKeyClassAlias);</b></p>\n";
@@ -5463,9 +5463,11 @@ abstract class MetaModel
 					continue; // Ignore this non-scalar value
 				}
 			}
-
-			$aSearches[] = '$'.$sSearch.'$';
-			$aReplacements[] = (string) $replace;
+			else
+			{
+				$aSearches[] = '$'.$sSearch.'$';
+				$aReplacements[] = (string) $replace;
+			}
 		}
 		return str_replace($aSearches, $aReplacements, $sInput);
 	}

core/mutex.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2013 Combodo SARL
+// Copyright (C) 2013-2016 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -24,7 +24,7 @@
  * Relies on MySQL locks because the API sem_get is not always present in the
  * installed PHP.    
  *
- * @copyright   Copyright (C) 2013 Combodo SARL
+ * @copyright   Copyright (C) 2013-2016 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 class iTopMutex
@@ -121,7 +121,41 @@ class iTopMutex
 			$this->bLocked = true;
 			self::$aAcquiredLocks[$this->sName]++;
 		}
-		return ($res === '1');
+		if (($res !== '1') && ($res !== '0'))
+		{
+			IssueLog::Error('GET_LOCK('.$this->sName.', 0) returned: '.var_export($res, true).'. Expected values are: 0, 1 or null !!');
+		}
+		return ($res !== '0');
+	}
+	
+	/**
+	 *	Check if the mutex is locked WITHOUT TRYING TO ACQUIRE IT
+	 *	@returns bool True if the mutex is in use, false otherwise
+	 */
+	public function IsLocked()
+	{
+		if ($this->bLocked)
+		{
+			return true; // Already acquired
+		}
+		if (self::$aAcquiredLocks[$this->sName] > 0)
+		{
+			return true;
+		}
+	
+		$res = $this->QueryToScalar("SELECT IS_FREE_LOCK('".$this->sName."')"); // IS_FREE_LOCK detects some error cases that IS_USED_LOCK do not detect
+		if (is_null($res))
+		{
+			$sMsg = "MySQL Error, IS_FREE_LOCK('".$this->sName."') returned null. Error (".mysqli_errno($this->hDBLink).") = '".mysqli_error($this->hDBLink)."'";
+			IssueLog::Error($sMsg);
+			throw new Exception($sMsg);
+		}
+		else if ($res == '1')
+		{
+			// Lock is free
+			return false;
+		}
+		return true;
 	}
 
 	/**

core/ormcaselog.class.inc.php

@@ -384,7 +384,7 @@ class ormCaseLog {
 	}
 
 
-	public function AddLogEntryFromJSON($oJson)
+	public function AddLogEntryFromJSON($oJson, $bCheckUserId = true)
 	{
 		$sText = isset($oJson->message) ? $oJson->message : '';
 
@@ -394,16 +394,24 @@ class ormCaseLog {
 			{
 				throw new Exception("Only administrators can set the user id", RestResult::UNAUTHORIZED);
 			}
-			try
+			if ($bCheckUserId)
 			{
-				$oUser = RestUtils::FindObjectFromKey('User', $oJson->user_id);
+				try
+				{
+					$oUser = RestUtils::FindObjectFromKey('User', $oJson->user_id);
+				}
+				catch(Exception $e)
+				{
+					throw new Exception('user_id: '.$e->getMessage(), $e->getCode());
+				}
+				$iUserId = $oUser->GetKey();
+				$sOnBehalfOf = $oUser->GetFriendlyName();
 			}
-			catch(Exception $e)
+			else
 			{
-				throw new Exception('user_id: '.$e->getMessage(), $e->getCode());
+				$iUserId = $oJson->user_id;
+				$sOnBehalfOf = $oJson->user_login;
 			}
-			$iUserId = $oUser->GetKey();
-			$sOnBehalfOf = $oUser->GetFriendlyName();
 		}
 		else
 		{

core/ormstopwatch.class.inc.php

@@ -86,8 +86,9 @@ class ormStopWatch
 	 * Get the working elapsed time since the start of the stop watch
 	 * even if it is currently running
 	 * @param oAttDef	AttributeDefinition Attribute hosting the stop watch
+	 * @param oObject	Hosting object (used for query parameters)
 	 */
-	public function GetElapsedTime($oAttDef)
+	public function GetElapsedTime($oAttDef, $oObject)
 	{
 		if (is_null($this->iLastStart))
 		{
@@ -95,7 +96,7 @@ class ormStopWatch
 		}
 		else
 		{
-			$iElapsed = $this->ComputeDuration($this, $oAttDef, $this->iLastStart, time());
+			$iElapsed = $this->ComputeDuration($oObject, $oAttDef, $this->iLastStart, time());
 			return $this->iTimeSpent + $iElapsed;
 		}
 	}

core/userrights.class.inc.php

@@ -400,7 +400,7 @@ abstract class UserInternal extends User
 		MetaModel::Init_InheritAttributes();
 
 		// When set, this token allows for password reset
-		MetaModel::Init_AddAttribute(new AttributeString("reset_pwd_token", array("allowed_values"=>null, "sql"=>"reset_pwd_token", "default_value"=>null, "is_null_allowed"=>true, "depends_on"=>array())));
+		MetaModel::Init_AddAttribute(new AttributeOneWayPassword("reset_pwd_token", array("allowed_values"=>null, "default_value"=>null, "is_null_allowed"=>true, "depends_on"=>array())));
 
 		// Display lists
 		MetaModel::Init_SetZListItems('details', array('contactid', 'first_name', 'email', 'login', 'language', 'profile_list', 'allowed_org_list')); // Attributes to be displayed for the complete details

datamodels/2.x/itop-attachments/main.attachments.php

@@ -250,6 +250,7 @@ EOF
 		$('#attachment_plugin').trigger('remove_attachment', [att_id]);
 		return false; // Do not submit the form !
 	}
+				
 	function ajaxFileUpload()
 	{
 		//starting setting some animation when the ajax starts and completes
@@ -368,6 +369,7 @@ $oPage->add_ready_script(
 		url: GetAbsoluteUrlModulesRoot()+'itop-attachments/ajax.attachment.php',
 		formData: { operation: 'add', temp_id: '$sTempId', obj_class: '$sClass' },
         dataType: 'json',
+		pasteZone: null, // Don't accept files via Chrome's copy/paste
         done: function (e, data) {
 			if(typeof(data.result.error) != 'undefined')
 			{
@@ -440,7 +442,6 @@ EOF
 );
 			$oPage->p('<span style="display:none;" id="attachment_loading">Loading, please wait...</span>');
 			$oPage->p('<input type="hidden" id="attachment_plugin" name="attachment_plugin"/>');
-			$oPage->add('</fieldset>');
 			if ($this->m_bDeleteEnabled)
 			{
 				$oPage->add_ready_script('$(".attachment").hover( function() {$(this).children(":button").toggleClass("btn_hidden"); } );');
@@ -466,7 +467,9 @@ EOF
 					$oPage->add('<div class="attachment" id="attachment_'.$iAttId.'"><a data-preview="'.$sPreview.'" href="'.$sDownloadLink.'"><img src="'.$sIcon.'"><br/>'.$sFileName.'</a><input type="hidden" name="attachments[]" value="'.$iAttId.'"/><br/>&nbsp;&nbsp;</div>');
 				}
 			}
+			$oPage->add('</span>');
 		}
+		$oPage->add('</fieldset>');
 		$sPreviewNotAvailable = addslashes(Dict::S('Attachments:PreviewNotAvailable'));
 		$iMaxWidth = MetaModel::GetModuleSetting('itop-attachments', 'preview_max_width', 290);
 		$oPage->add_ready_script("$(document).tooltip({ items: '.attachment a',  position: { my: 'left top', at: 'right top', using: function( position, feedback ) { $( this ).css( position ); }}, content: function() { if ($(this).attr('data-preview') == 'true') { return('<img style=\"max-width:{$iMaxWidth}px\" src=\"'+$(this).attr('href')+'\"></img>');} else { return '$sPreviewNotAvailable'; }}});");

datamodels/2.x/itop-backup/ajax.backup.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2014 Combodo SARL
+// Copyright (C) 2013-2016 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -18,8 +18,7 @@
 
 /**
  * Backup from an interactive session
- *
- * @copyright   Copyright (C) 2013 Combodo SARL
+ * @copyright   Copyright (C) 2013-2016 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -77,9 +76,8 @@ try
 
 		$sEnvironment = utils::ReadParam('environment', 'production', false, 'raw_data');
 		$oRestoreMutex = new iTopMutex('restore.'.$sEnvironment);
-		if ($oRestoreMutex->TryLock())
+		if (!$oRestoreMutex->IsLocked())
 		{
-			$oRestoreMutex->Unlock();
 			$sFile = utils::ReadParam('file', '', false, 'raw_data');
 			$sToken = str_replace(' ', '', (string)microtime());
 			$sTokenFile = APPROOT.'/data/restore.'.$sToken.'.tok';

datamodels/2.x/itop-backup/main.itop-backup.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2014 Combodo SARL
+// Copyright (C) 2014-2016 Combodo SARL
 //
 //   This program is free software; you can redistribute it and/or modify
 //   it under the terms of the GNU General Public License as published by
@@ -29,24 +29,8 @@ class BackupHandler extends ModuleHandlerAPI
 
 		try
 		{
-			$oBackupMutex = new iTopMutex('backup.'.utils::GetCurrentEnvironment());
-			if ($oBackupMutex->TryLock())
-			{
-				$oBackupMutex->Unlock();
-			}
-			else
-			{
-				// Not needed: the DB dump is done in a single transaction
-				//MetaModel::GetConfig()->Set('access_mode', ACCESS_READONLY, 'itop-backup');
-				//MetaModel::GetConfig()->Set('access_message', ' - '.dict::S('bkp-backup-running'), 'itop-backup');
-			}
-	
 			$oRestoreMutex = new iTopMutex('restore.'.utils::GetCurrentEnvironment());
-			if ($oRestoreMutex->TryLock())
-			{
-				$oRestoreMutex->Unlock();
-			}
-			else
+			if ($oRestoreMutex->IsLocked())
 			{
 				MetaModel::GetConfig()->Set('access_mode', ACCESS_READONLY, 'itop-backup');
 				MetaModel::GetConfig()->Set('access_message', ' - '.dict::S('bkp-restore-running'), 'itop-backup');

datamodels/2.x/itop-backup/status.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2014 Combodo SARL
+// Copyright (C) 2016 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -20,7 +20,7 @@
 /**
  * Monitor the backup
  *
- * @copyright   Copyright (C) 2013 Combodo SARL
+ * @copyright   Copyright (C) 2016 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -169,14 +169,13 @@ try
 	}
 
 	$oRestoreMutex = new iTopMutex('restore.'.utils::GetCurrentEnvironment());
-	if ($oRestoreMutex->TryLock())
+	if ($oRestoreMutex->IsLocked())
 	{
-		$oRestoreMutex->Unlock();
-		$sDisableRestore = '';
+		$sDisableRestore = 'disabled="disabled"';
 	}
 	else
 	{
-		$sDisableRestore = 'disabled="disabled"';
+		$sDisableRestore = '';
 	}
 	
 	// 1st table: list the backups made in the background
@@ -271,20 +270,12 @@ try
 	// Ongoing operation ?
 	//
 	$oBackupMutex = new iTopMutex('backup.'.utils::GetCurrentEnvironment());
-	if ($oBackupMutex->TryLock())
-	{
-		$oBackupMutex->Unlock();
-	}
-	else
+	if ($oBackupMutex->IsLocked())
 	{
 		$oP->p(Dict::S('bkp-backup-running'));
 	}
 	$oRestoreMutex = new iTopMutex('restore.'.utils::GetCurrentEnvironment());
-	if ($oRestoreMutex->TryLock())
-	{
-		$oRestoreMutex->Unlock();
-	}
-	else
+	if ($oRestoreMutex->IsLocked())
 	{
 		$oP->p(Dict::S('bkp-restore-running'));
 	}

datamodels/2.x/itop-config/config.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2014 Combodo SARL
+// Copyright (C) 2014-2016 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -105,6 +105,10 @@ try
 	{
 		$oP->add("<div class=\"header_message message_info\">Sorry, iTop is in <b>demonstration mode</b>: the configuration file cannot be edited.</div>");
 	}
+	if (MetaModel::GetModuleSetting('itop-config', 'config_editor', '') == 'disabled')
+	{
+		$oP->add("<div class=\"header_message message_info\">iTop interactive edition of the configuration as been disabled. See <tt>'config_editor' => 'disabled'</tt> in the configuration file.</div>");
+	}
 	else
 	{
 		$oP->add_style(
@@ -129,27 +133,50 @@ EOF
 		if ($sOperation == 'save')
 		{
 			$sConfig = utils::ReadParam('new_config', '', false, 'raw_data');
+			$sTransactionId = utils::ReadParam('transaction_id', '');
 			$sOrginalConfig = utils::ReadParam('prev_config', '', false, 'raw_data');
-			if ($sConfig == $sOrginalConfig)
+			if (!utils::IsTransactionValid($sTransactionId, true))
 			{
-				$oP->add('<div id="save_result" class="header_message">'.Dict::S('config-no-change').'</div>');
+				$oP->add("<div class=\"header_message message_info\">Error: invalid Transaction ID. The configuration was <b>NOT</b> modified.</div>");
 			}
 			else
 			{
-				try
+				if ($sConfig == $sOrginalConfig)
 				{
-					TestConfig($sConfig, $oP); // throws exceptions
-		
-					@chmod($sConfigFile, 0770); // Allow overwriting the file
-					file_put_contents($sConfigFile, $sConfig);
-					@chmod($sConfigFile, 0444); // Read-only
-		
-					$oP->p('<div id="save_result" class="header_message message_ok">'.Dict::S('Successfully recorded.').'</div>');
-					$sOrginalConfig = str_replace("\r\n", "\n", file_get_contents($sConfigFile));
+					$oP->add('<div id="save_result" class="header_message">'.Dict::S('config-no-change').'</div>');
 				}
-				catch (Exception $e)
+				else
 				{
-					$oP->p('<div id="save_result" class="header_message message_error">'.$e->getMessage().'</div>');
+					try
+					{
+						TestConfig($sConfig, $oP); // throws exceptions
+			
+						@chmod($sConfigFile, 0770); // Allow overwriting the file
+						$sTmpFile = tempnam(SetupUtils::GetTmpDir(), 'itop-cfg-');
+						// Don't write the file as-is since it would allow to inject any kind of PHP code.
+						// Instead write the interpreted version of the file
+						// Note:
+						// The actual raw PHP code will anyhow be interpreted exactly twice: once in TestConfig() above
+						// and a second time during the load of the Config object below.
+						// If you are really concerned about an iTop administrator crafting some malicious
+						// PHP code inside the config file, then turn off the interactive configuration
+						// editor by adding the configuration parameter:
+						// 'itop-config' => array(
+						//     'config_editor' => 'disabled',
+						// )
+						file_put_contents($sTmpFile, $sConfig);
+						$oTempConfig = new Config($sTmpFile, true);
+						$oTempConfig->WriteToFile($sConfigFile);
+						@unlink($sTmpFile);
+						@chmod($sConfigFile, 0444); // Read-only
+			
+						$oP->p('<div id="save_result" class="header_message message_ok">'.Dict::S('Successfully recorded.').'</div>');
+						$sOrginalConfig = str_replace("\r\n", "\n", file_get_contents($sConfigFile));
+					}
+					catch (Exception $e)
+					{
+						$oP->p('<div id="save_result" class="header_message message_error">'.$e->getMessage().'</div>');
+					}
 				}
 			}
 		}
@@ -164,6 +191,7 @@ EOF
 		$oP->p(Dict::S('config-edit-intro'));
 		$oP->add("<form method=\"POST\">");
 		$oP->add("<input type=\"hidden\" name=\"operation\" value=\"save\">");
+		$oP->add("<input type=\"hidden\" name=\"transaction_id\" value=\"".utils::GetNewTransactionId()."\">");
 		$oP->add("<input type=\"submit\" value=\"".Dict::S('config-apply')."\"><button onclick=\"ResetConfig(); return false;\">".Dict::S('config-cancel')."</button>");
 		$oP->add("<span class=\"current_line\">".Dict::Format('config-current-line', "<span class=\"line_number\"></span>")."</span>");
 		$oP->add("<input type=\"hidden\" id=\"prev_config\" name=\"prev_config\" value=\"$sOriginalConfigEscaped\">");

js/linksdirectwidget.js

@@ -238,8 +238,12 @@ $(function()
 				}
 			);
 			oParams.operation = 'searchObjectsToAdd2';
-			oParams['class'] = this.options.class_name;
 			oParams.real_class = '';
+			if ((oParams['class'] != undefined) && (oParams['class'] != ''))
+			{
+				oParams.real_class = oParams['class'];				
+			}
+			oParams['class'] = this.options.class_name;
 			oParams.att_code = this.options.att_code;
 			oParams.iInputId = this.id;
 			if (this.options.oWizardHelper)

js/utils.js

@@ -200,13 +200,18 @@ function ReloadSearchForm(divId, sClassName, sBaseClass, sContext)
 				for(var index = 0; index < aSubmit.length; index++)
 				{
 					// Restore the previously bound submit handlers
+					var sEventName = 'submit';
+					if ((aSubmit[index].namespace != undefined) && (aSubmit[index].namespace != ''))
+					{
+						sEventName += '.'+aSubmit[index].namespace;
+					}
 					if (aSubmit[index].data != undefined)
 					{
-						oForm.bind('submit.'+aSubmit[index].namespace, aSubmit[index].data, aSubmit[index].handler)
+						oForm.bind(sEventName, aSubmit[index].data, aSubmit[index].handler)
 					}
 					else
 					{
-						oForm.bind('submit.'+aSubmit[index].namespace, aSubmit[index].handler)
+						oForm.bind(sEventName, aSubmit[index].handler)
 					}
 				}
 		   }

pages/UI.php

@@ -601,6 +601,7 @@ try
 				if ($iErrors == 0)
 				{
 					$oP->set_title(Dict::S('UI:SearchResultsPageTitle'));
+					$oP->add_linked_script(utils::GetAbsoluteUrlAppRoot().'js/xlsx-export.js'); // Since the results are loaded asynchronously they don't do this on their own
 					$oP->add("<div style=\"padding: 10px;\">\n");
 					$oP->add("<div class=\"header_message\" id=\"full_text_progress\" style=\"position: fixed; background-color: #cccccc; opacity: 0.7; padding: 1.5em;\">\n");
 					$oP->add('<img id="full_text_indicator" src="../images/indicator.gif">&nbsp;<span style="padding: 1.5em;">'.Dict::Format('UI:Search:Ongoing', htmlentities($sFullText, ENT_QUOTES, 'UTF-8')).'</span>');
@@ -1654,4 +1655,4 @@ catch(Exception $e)
 		IssueLog::Error($e->getMessage());
 	}
 }
-?>
+?>

setup/wizardsteps.class.inc.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2012 Combodo SARL
+// Copyright (C) 2010-2016 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -18,7 +18,7 @@
 
 /**
  * All the steps of the iTop installation wizard
- * @copyright   Copyright (C) 2010-2012 Combodo SARL
+ * @copyright   Copyright (C) 2010-2016 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -623,11 +623,7 @@ EOF
 				$this->oWizard->GetParameter('db_user', ''),
 				$this->oWizard->GetParameter('db_pwd', '')
 			);
-			if ($oMutex->TryLock())
-			{
-				$oMutex->Unlock();
-			}
-			else
+			if ($oMutex->IsLocked())
 			{
 				$oPage->p("<img src=\"../images/error.png\"/>&nbsp;An iTop CRON process is being executed on the target database. It is highly recommended to stop any iTop CRON process prior to running the setup program.");
 			}