CVE-2021-41245 (PHP, published 2022-04-05T15:15Z)
Description: value: Combodo iTop is a web based IT Service Management tool. In versions prior to 2.7.6 and 3.0.0, CSRF tokens generated by `privUITransactionFile` aren't properly checked. Versions 2.7.6 and 3.0.0 contain a patch for this issue. As a workaround, use the session implementation by adding in the iTop config file.
Severity: MEDIUM (CVSS v3 base score: 8.1)
Weakness: CWE-352 \u2014 Cross-Site Request Forgery (CSRF)
The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
Repository: https://github.com/Combodo/iTop
Files changed:
application/transaction.class.inc.php
core/userrights.class.inc.php
test/application/privUITransactionFileTest.php
Commit message: N°4289 Security hardening


Vulnerable code:
		$id = basename(tempnam(APPROOT.'data/transactions', static::GetUserPrefix()));
		self::Info('GetNewTransactionId: Created transaction: '.$id);
		return (string)$id;
		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()));
		}
		$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"
		} else {
			$bSuccess = @unlink($sFilepath);
		}

		if (!$bSuccess) {
			self::Error('RemoveTransaction: FAILED to remove transaction '.$id);
		} else {
			self::Info('RemoveTransaction: OK '.$id);
		return $bSuccess;
			if (array_key_exists('profile_list', $_SESSION))

Fixed code:
	/**
	 * @return int
	 * @throws \SecurityException if no connected user
	 *
	 * @since 2.6.5 2.7.6 3.0.0 N°4289 method creation
	 */
	private static function GetCurrentUserId() {
		$iCurrentUserId = UserRights::GetConnectedUserId();
		if ('' === $iCurrentUserId) {
			throw new SecurityException('Cannot creation transaction_id when no user logged');
		}

		return $iCurrentUserId;
	}

	 *
	 *
	 *
	 * @throws \SecurityException
	 * @throws \Exception
	 *
	 * @since 2.6.5 2.7.6 3.0.0 security hardening + throws SecurityException if no user logged


		$iCurrentUserId = static::GetCurrentUserId();

		$sTransactionIdFullPath = tempnam(APPROOT.'data/transactions', static::GetUserPrefix());
		file_put_contents($sTransactionIdFullPath, $iCurrentUserId, LOCK_EX);

		$sTransactionIdFileName = basename($sTransactionIdFullPath);
		self::Info('GetNewTransactionId: Created transaction: '.$sTransactionIdFileName);

		return $sTransactionIdFileName;
	 *
	 *
	 *
	 * @since 2.6.5 2.7.6 3.0.0 N°4289 security hardening

		if (false === $bResult) {
			self::Info("IsTransactionValid: Transaction '$id' not found. Pending transactions:\n".implode("\n", self::GetPendingTransactions()));
			return false;
		}

		$iCurrentUserId = static::GetCurrentUserId();
		$sTransactionIdUserId = file_get_contents($sFilepath);
		if ($iCurrentUserId != $sTransactionIdUserId) {
			self::Info("IsTransactionValid: Transaction '$id' not existing for current user. Pending transactions:\n".implode("\n", self::GetPendingTransactions()));
			return false;
		}

		if ($bRemoveTransaction)
			$bResult = @unlink($sFilepath);
			if (!$bResult)
				self::Error('IsTransactionValid: FAILED to remove transaction '.$id);
			}
			else
			{
				self::Info('IsTransactionValid: OK. Removed transaction: '.$id);

	 *
	 * @since 2.6.5 2.7.6 3.0.0 N°4289 security hardening
		/** @noinspection PhpRedundantOptionalArgumentInspection */
		$bResult = static::IsTransactionValid($id, true);
		if (false === $bResult) {
			self::Error("RemoveTransaction: Transaction '$id' is invalid. Pending transactions:\n"
			return false;
		return true;
	/**
	 * @return int|string ID of the connected user : if impersonate then use {@see m_oRealUser}, else {@see m_oUser}. If no user set then return ''
	 * @since 2.6.5 2.7.6 3.0.0 N°4289 method creation
	 */
	public static function GetConnectedUserId() {
		if (false === is_null(static::$m_oRealUser)) {
			return static::$m_oRealUser->GetKey();
		}
		if (false === is_null(static::$m_oUser)) {
			return static::$m_oUser->GetKey();
		}

		return '';
	}

			if ((false === utils::IsModeCLI()) && array_key_exists('profile_list', $_SESSION))
<?php
class privUITransactionFileTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase
{
	const USER_TEST_LOGIN = 'support_test_privUITransaction';

	public function testIsTransactionValid() {
		$this->CreateUser(static::USER_TEST_LOGIN, 5); // profile:5 is "Support agent"

		// create token in the support user context
		UserRights::Login(self::USER_TEST_LOGIN);
		$sTransactionIdUserSupport = privUITransactionFile::GetNewTransactionId();
		$bResult = privUITransactionFile::IsTransactionValid($sTransactionIdUserSupport, false);
		$this->assertTrue($bResult, 'Token created by support user must be valid in the support user context');

		// test token in the admin user context
		UserRights::Login('admin');
		$bResult = privUITransactionFile::IsTransactionValid($sTransactionIdUserSupport, false);
		$this->assertFalse($bResult, 'Token created by support user must be invalid in the admin user context');
		$bResult = privUITransactionFile::RemoveTransaction($sTransactionIdUserSupport);
		$this->assertFalse($bResult, 'Token created by support user cannot be removed in the admin user context');

		// test other methods in the support user context
		UserRights::Login(self::USER_TEST_LOGIN);
		$bResult = privUITransactionFile::RemoveTransaction($sTransactionIdUserSupport);
		$this->assertTrue($bResult, 'Token created by support user must be removed in the support user context');
	}
}