From 95a0efedcf39d2c85603d10db7119241fca80cc1 Mon Sep 17 00:00:00 2001
From: bruno-ds <bruno.dasilva@combodo.com>
Date: Mon, 1 Mar 2021 15:28:34 +0100
Subject: [PATCH] =?UTF-8?q?N=C2=B03728=20-=20security=20hardening?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 application/transaction.class.inc.php         |   6 +-
 .../application/privUITransactionFileTest.php | 135 ++++++++++++++++++
 2 files changed, 138 insertions(+), 3 deletions(-)
 create mode 100644 test/application/privUITransactionFileTest.php

diff --git a/application/transaction.class.inc.php b/application/transaction.class.inc.php
index eac5ef6c9..d5388976f 100644
--- a/application/transaction.class.inc.php
+++ b/application/transaction.class.inc.php
@@ -297,11 +297,11 @@ class privUITransactionFile
 	 * 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()
+	protected static function CleanupOldTransactions($sTransactionDir = null)
 	{
 		$iLimit = time() - 24*3600;
-		clearstatcache();
-		$aTransactions = glob(APPROOT.'data/transactions/*-*');
+		$sPattern = $sTransactionDir ? "$sTransactionDir/*" : APPROOT.'data/transactions/*';
+		$aTransactions = glob($sPattern);
 		foreach($aTransactions as $sFileName)
 		{
 			if (filemtime($sFileName) < $iLimit)
diff --git a/test/application/privUITransactionFileTest.php b/test/application/privUITransactionFileTest.php
new file mode 100644
index 000000000..d15c20ad5
--- /dev/null
+++ b/test/application/privUITransactionFileTest.php
@@ -0,0 +1,135 @@
+<?php
+/**
+ * Copyright (C) 2013-2020 Combodo SARL
+ *
+ * This file is part of iTop.
+ *
+ * iTop is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * iTop is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ */
+
+use Combodo\iTop\Test\UnitTest\ItopTestCase;
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ * @covers utils
+ */
+class privUITransactionFileTest extends ItopTestCase
+{
+	public function setUp()
+	{
+		parent::setUp();
+		require_once(APPROOT.'/application/startup.inc.php');
+//		require_once(APPROOT.'application/utils.inc.php');
+	}
+
+	/**
+	 * @dataProvider cleanupOldTransactionsProvider
+	 */
+	public function testCleanupOldTransactions($iCleanableCreated, $iPreservableCreated, $sCleanablePrefix, $sPreservablePrefix)
+	{
+//		MetaModel::GetConfig()->Set('transactions_gc_threshold', 100);
+
+		$iBaseLimit = time() - 24*3600; //24h
+
+		$sBaseDir = sys_get_temp_dir();
+		$sDir = "$sBaseDir/privUITransactionFileTest/cleanupOldTransactions";
+		if (is_dir($sDir)) {
+			$this->rm($sDir);
+		}
+		mkdir("$sDir", 0777, true);
+
+		for ($i = 0; $i < $iCleanableCreated; $i++) {
+			touch("$sDir/{$sCleanablePrefix}$i", $iBaseLimit - 10*60);
+		}
+		for ($i = 0; $i < $iPreservableCreated; $i++) {
+			touch("$sDir/{$sPreservablePrefix}$i", $iBaseLimit + 10*60);
+		}
+
+		$iCleanableCount = count(glob("$sDir/{$sCleanablePrefix}*"));
+		$iPreservableCount = count(glob("$sDir/{$sPreservablePrefix}*"));
+		$this->assertEquals($iCleanableCreated, $iCleanableCount);
+		$this->assertEquals($iPreservableCreated, $iPreservableCount);
+
+		$aArgs = [
+			'sTransactionDir' => "$sDir",
+		];
+		$oprivUITransactionFile = new privUITransactionFile();
+		$this->InvokeNonPublicMethod(get_class($oprivUITransactionFile), 'CleanupOldTransactions', $oprivUITransactionFile, $aArgs);
+
+		$iCleanableCount = count(glob("$sDir/{$sCleanablePrefix}*"));
+		$iPreservableCount = count(glob("$sDir/{$sPreservablePrefix}*"));
+		$this->assertEquals(0, $iCleanableCount);
+		$this->assertEquals($iPreservableCreated, $iPreservableCount);
+	}
+
+	public function cleanupOldTransactionsProvider()
+	{
+		$iBaseLimit = time() - 60 * 10; //ten minutes ago
+
+		$sBaseDir = sys_get_temp_dir();
+		$sDir = "$sBaseDir/privUITransactionFileTest/cleanupOldTransactions";
+
+		return  [
+			'linux - no content' => [
+				'iCleanableCreated' => 0,
+				'iPreservableCreated' => 0,
+				'sCleanablePrefix' => 'cleanable-',
+				'sPreservablePrefix' => 'preservable-',
+			],
+			'linux - cleanable content' => [
+				'iCleanableCreated' => 2,
+				'iPreservableCreated' => 0,
+				'sCleanablePrefix' => 'cleanable-',
+				'sPreservablePrefix' => 'preservable-',
+			],
+			'linux - preseved content' => [
+				'iCleanableCreated' => 0,
+				'iPreservableCreated' => 2,
+				'sCleanablePrefix' => 'cleanable-',
+				'sPreservablePrefix' => 'preservable-',
+			],
+			'win - no content' => [
+				'iCleanableCreated' => 0,
+				'iPreservableCreated' => 0,
+				'sCleanablePrefix' => 'cle',
+				'sPreservablePrefix' => 'pre',
+			],
+			'win - cleanable content' => [
+				'iCleanableCreated' => 2,
+				'iPreservableCreated' => 0,
+				'sCleanablePrefix' => 'cle',
+				'sPreservablePrefix' => 'pre',
+			],
+			'win - preseved content' => [
+				'iCleanableCreated' => 0,
+				'iPreservableCreated' => 2,
+				'sCleanablePrefix' => 'cle',
+				'sPreservablePrefix' => 'pre',
+			],
+		];
+	}
+
+	public function rm($sDir) {
+		$aFiles = array_diff(scandir($sDir), ['.','..']);
+		foreach ($aFiles as $sFile) {
+			if ((is_dir("$sDir/$sFile"))) {
+				$this->rm("$sDir/$sFile");
+			} else {
+				unlink("$sDir/$sFile");
+			}
+		}
+		return rmdir($sDir);
+	}
+}