From d7e5705520a13406951f862e3d420a9e9a975edd Mon Sep 17 00:00:00 2001
From: Anne-Catherine <57360138+accognet@users.noreply.github.com>
Date: Fri, 12 Aug 2022 17:50:14 +0200
Subject: [PATCH] =?UTF-8?q?N=C2=B05318=20-=20Fix=20DBObject::CheckValue()?=
 =?UTF-8?q?=20messages=20being=20HTML=20encoded=20when=20not=20necessary?=
 =?UTF-8?q?=20(#326)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Rollback N°4898 - Security fix

* N°5318 - security fix
---
 application/cmdbabstract.class.inc.php |  6 +++++-
 core/coreexception.class.inc.php       | 16 ++++++----------
 core/dbobject.class.php                | 12 ++++++------
 pages/UI.php                           |  5 ++++-
 4 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/application/cmdbabstract.class.inc.php b/application/cmdbabstract.class.inc.php
index 1139a08df..9f4188c18 100644
--- a/application/cmdbabstract.class.inc.php
+++ b/application/cmdbabstract.class.inc.php
@@ -4735,11 +4735,15 @@ EOF
 			$sCSSClass = $bResult ? HILIGHT_CLASS_NONE : HILIGHT_CLASS_CRITICAL;
 			$sChecked = $bResult ? 'checked' : '';
 			$sDisabled = $bResult ? '' : 'disabled';
+
+			$aErrorsToDisplay = array_map(function($sError) {
+				return utils::HtmlEntities($sError);
+			}, $aErrors);
 			$aRows[] = array(
 				'form::select' => "<input type=\"checkbox\" class=\"selectList\" $sChecked $sDisabled\"></input>",
 				'object' => $oObj->GetHyperlink(),
 				'status' => $sStatus,
-				'errors' => '<p>'.($bResult ? '' : implode('</p><p>', $aErrors)).'</p>',
+				'errors' => '<p>'.($bResult ? '' : implode('</p><p>', $aErrorsToDisplay)).'</p>',
 				'@class' => $sCSSClass,
 			);
 			if ($bResult && (!$bPreview))
diff --git a/core/coreexception.class.inc.php b/core/coreexception.class.inc.php
index a85417786..b29ece9b1 100644
--- a/core/coreexception.class.inc.php
+++ b/core/coreexception.class.inc.php
@@ -165,19 +165,15 @@ class CoreCannotSaveObjectException extends CoreException
 	public function getHtmlMessage()
 	{
 		$sTitle = Dict::S('UI:Error:SaveFailed');
-		$sContent = "<span><strong>{$sTitle}</strong></span>";
+		$sContent = "<span><strong>".utils::HtmlEntities($sTitle)."</strong></span>";
 
-		if (count($this->aIssues) == 1)
-		{
+		if (count($this->aIssues) == 1) {
 			$sIssue = reset($this->aIssues);
-			$sContent .= " <span>{$sIssue}</span>";
-		}
-		else
-		{
+			$sContent .= " <span>".utils::HtmlEntities($sIssue)."</span>";
+		} else {
 			$sContent .= '<ul>';
-			foreach ($this->aIssues as $sError)
-			{
-				$sContent .= "<li>$sError</li>";
+			foreach ($this->aIssues as $sError) {
+				$sContent .= "<li>".utils::HtmlEntities($sError)."</li>";
 			}
 			$sContent .= '</ul>';
 		}
diff --git a/core/dbobject.class.php b/core/dbobject.class.php
index dcd073638..dc2fd1484 100644
--- a/core/dbobject.class.php
+++ b/core/dbobject.class.php
@@ -1880,7 +1880,7 @@ abstract class DBObject implements iDisplay
 				$oTargetObj = MetaModel::GetObject($sTargetClass, $toCheck, false /*must be found*/, true /*allow all data*/);
 				if (is_null($oTargetObj))
 				{
-					return "Target object not found (".utils::HtmlEntities($sTargetClass).".::".utils::HtmlEntities($toCheck).")";
+					return "Target object not found ($sTargetClass::$toCheck)";
 				}
 			}
 			if ($oAtt->IsHierarchicalKey())
@@ -1889,7 +1889,7 @@ abstract class DBObject implements iDisplay
 				$aValues = $oAtt->GetAllowedValues(array('this' => $this));
 				if (!array_key_exists($toCheck, $aValues))
 				{
-					return "Value not allowed [". utils::HtmlEntities($toCheck)."]";
+					return "Value not allowed [$toCheck]";
 				}
 			}
 		}
@@ -1903,7 +1903,7 @@ abstract class DBObject implements iDisplay
 					$oTag->SetValues(explode(' ', $toCheck));
 				} catch (Exception $e)
 				{
-					return "Tag value [". utils::HtmlEntities($toCheck)."] is not a valid tag list";
+					return "Tag value '$toCheck' is not a valid tag list";
 				}
 
 				return true;
@@ -1931,7 +1931,7 @@ abstract class DBObject implements iDisplay
 					$oTag->SetValues($aValues);
 				} catch (Exception $e)
 				{
-					return "Set value[". utils::HtmlEntities($toCheck)."] is not a valid set";
+					return "Set value '$toCheck' is not a valid set";
 				}
 
 				return true;
@@ -1951,7 +1951,7 @@ abstract class DBObject implements iDisplay
 			{
 				if (!array_key_exists($toCheck, $aValues))
 				{
-					return "Value not allowed [". utils::HtmlEntities($toCheck)."]";
+					return "Value not allowed [$toCheck]";
 				}
 			}
 			if (!is_null($iMaxSize = $oAtt->GetMaxSize()))
@@ -1964,7 +1964,7 @@ abstract class DBObject implements iDisplay
 			}
 			if (!$oAtt->CheckFormat($toCheck))
 			{
-				return "Wrong format [". utils::HtmlEntities($toCheck)."]";
+				return "Wrong format [$toCheck]";
 			}
 		}
 		else
diff --git a/pages/UI.php b/pages/UI.php
index 8c1bdf057..1d464904c 100644
--- a/pages/UI.php
+++ b/pages/UI.php
@@ -1614,7 +1614,10 @@ EOF
 								}
 								else
 								{
-									$sError = '<p>'.implode('</p></p>',$aErrors)."</p>\n";
+									$aErrorsToDisplay = array_map(function($sError) {
+										return utils::HtmlEntities($sError);
+									}, $aErrors);
+									$sError = '<p>'.implode('</p></p>',$aErrorsToDisplay)."</p>\n";
 								}
 							}
 							else