From 5651512f689c2168391a029515690232d0db7c57 Mon Sep 17 00:00:00 2001
From: Molkobain <lajarige.guillaume@free.fr>
Date: Tue, 17 Aug 2021 15:25:43 +0200
Subject: [PATCH] =?UTF-8?q?N=C2=B04244=20-=20Add=20protection=20against=20?=
 =?UTF-8?q?unfortunate=20massive=20delete=20of=20inline=20images=20/=20att?=
 =?UTF-8?q?achments=20when=20a=20null=20temp=20ID=20is=20passed?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 core/inlineimage.class.inc.php                | 17 ++++-
 .../main.itop-attachments.php                 |  5 ++
 test/core/InlineImageTest.php                 | 68 +++++++++++++++++++
 3 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 test/core/InlineImageTest.php

diff --git a/core/inlineimage.class.inc.php b/core/inlineimage.class.inc.php
index f76d8aef1..a454170d3 100644
--- a/core/inlineimage.class.inc.php
+++ b/core/inlineimage.class.inc.php
@@ -229,7 +229,7 @@ class InlineImage extends DBObject
 	 *
 	 * @param string $sTempId
 	 *
-	 * @return void
+	 * @return bool True if cleaning was successful, false if anything aborted it
 	 * @throws \ArchivedObjectException
 	 * @throws \CoreCannotSaveObjectException
 	 * @throws \CoreException
@@ -239,8 +239,19 @@ class InlineImage extends DBObject
 	 * @throws \MySQLHasGoneAwayException
 	 * @throws \OQLException
 	 */
-	public static function OnFormCancel($sTempId)
+	public static function OnFormCancel($sTempId): bool
 	{
+		// Protection against unfortunate massive delete of inline images when a null temp ID is passed
+		if (strlen($sTempId) === 0) {
+			IssueLog::Trace('OnFormCancel "error" $sTempId is null or empty', LogChannels::INLINE_IMAGE, array(
+				'$sTempId' => $sTempId,
+				'$sUser' => UserRights::GetUser(),
+				'HTTP_REFERER' => @$_SERVER['HTTP_REFERER'],
+			));
+
+			return false;
+		}
+
 		// Delete all "pending" InlineImages for this form
 		$sOQL = 'SELECT InlineImage WHERE temp_id = :temp_id';
 		$oSearch = DBObjectSearch::FromOQL($sOQL);
@@ -257,6 +268,8 @@ class InlineImage extends DBObject
 			'$sUser' => UserRights::GetUser(),
 			'HTTP_REFERER' => @$_SERVER['HTTP_REFERER'],
 		));
+
+		return true;
 	}
 
 	/**
diff --git a/datamodels/2.x/itop-attachments/main.itop-attachments.php b/datamodels/2.x/itop-attachments/main.itop-attachments.php
index a2f4e0c0d..24ad38a6e 100644
--- a/datamodels/2.x/itop-attachments/main.itop-attachments.php
+++ b/datamodels/2.x/itop-attachments/main.itop-attachments.php
@@ -111,6 +111,11 @@ class AttachmentPlugIn implements iApplicationUIExtension, iApplicationObjectExt
 
 	public function OnFormCancel($sTempId)
 	{
+		// Protection against unfortunate massive delete of attachments when a null temp ID is passed
+		if (strlen($sTempId) === 0) {
+			return;
+		}
+
 		// Delete all "pending" attachments for this form
 		$sOQL = 'SELECT Attachment WHERE temp_id = :temp_id';
 		$oSearch = DBObjectSearch::FromOQL($sOQL);
diff --git a/test/core/InlineImageTest.php b/test/core/InlineImageTest.php
new file mode 100644
index 000000000..f77b01c01
--- /dev/null
+++ b/test/core/InlineImageTest.php
@@ -0,0 +1,68 @@
+<?php
+/*
+ * @copyright   Copyright (C) 2010-2021 Combodo SARL
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use InlineImage;
+
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ */
+class InlineImageTest extends ItopDataTestCase
+{
+	/**
+	 * @dataProvider OnFormCancelInvalidTempIdProvider
+	 *
+	 * @param $sTempId
+	 * @param bool $bExpectedReturn
+	 *
+	 * @throws \ArchivedObjectException
+	 * @throws \CoreCannotSaveObjectException
+	 * @throws \CoreException
+	 * @throws \CoreUnexpectedValue
+	 * @throws \DeleteException
+	 * @throws \MySQLException
+	 * @throws \MySQLHasGoneAwayException
+	 * @throws \OQLException
+	 * @covers       InlineImage::OnFormCancel()
+	 */
+	public function testOnFormCancelInvalidTempId($sTempId, bool $bExpectedReturn)
+	{
+		$bTestReturn = InlineImage::OnFormCancel($sTempId);
+		$this->assertEquals($bExpectedReturn, $bTestReturn);
+	}
+
+	public function OnFormCancelInvalidTempIdProvider()
+	{
+		return [
+			'Null temp_id' => [
+				null,
+				false,
+			],
+			'Empty temp_id' => [
+				'',
+				false,
+			],
+			'0 as integer temp_id' => [
+				0,
+				true,
+			],
+			'0 as string temp_id' => [
+				'0',
+				true,
+			],
+			'String temp_id' => [
+				'fake_temp_id',
+				true,
+			],
+		];
+	}
+}