N°4127 - Security: Fix XSS vulnerability in object attribute's tooltip

application/cmdbabstract.class.inc.php

@@ -2533,14 +2533,13 @@ EOF
 						$sDisplayValueForHtml = utils::EscapeHtml($sDisplayValue);
 
 						// Adding tooltip so we can read the whole value when its very long (eg. URL)
-						$sTip = '';
+                        $sTip = '';
 						if (!empty($sDisplayValue)) {
 							$sTip = 'data-tooltip-content="'.$sDisplayValueForHtml.'"';
-							$oPage->add_ready_script(
-								<<<EOF
+							$oPage->add_ready_script(<<<JS
 								$('#{$iId}').on('keyup', function(evt, sFormId){ 
-									var sVal = $('#{$iId}').val();
-									var oTippy = this._tippy;
+									let sVal = $('#{$iId}').val();
+									const oTippy = this._tippy;
 									
 									if(sVal === '')
 									{
@@ -2553,7 +2552,7 @@ EOF
 									}
 									oTippy.setContent(sVal);
 								});
-EOF
+JS
 							);
 						}
 

css/backoffice/vendors/_all.scss

@@ -19,6 +19,7 @@
 @import "bulma-variables-overload";
 @import "../../../node_modules/bulma-scss/bulma";
 @import "ckeditor";
+@import "tippy";
 @import "jqueryui";
 @import "jquery-multiselect";
 @import "datatables";

css/backoffice/vendors/_tippy.scss

@@ -0,0 +1,9 @@
+/*!
+ * @copyright   Copyright (C) 2010-2021 Combodo SARL
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+
+// Allow plain text (opposite of HTML) multi-lines tooltips to be displayed correctly, otherwise it will all be on a single line.
+.tippy-content {
+  white-space: pre-line;
+}

js/utils.js

@@ -709,6 +709,22 @@ const CombodoGlobalToolbox = {
 				|| oDOMElem.contains(efp(oRect.left, oRect.bottom))
 			);
 		}
+	},
+	/**
+	 * This method should be a JS mirror of the PHP {@see utils::FilterXSS} method
+	 *
+	 * @param sInput {string} Input text to filter from XSS attacks
+	 * @returns {string} The sInput string filtered from possible XSS attacks
+	 * @constructor
+	 * @since 3.0.0
+	 */
+	FilterXSS: function (sInput) {
+		let sOutput = sInput;
+
+		// Remove HTML script tags
+		sOutput = sOutput.replace(/<script/g, '&lt;script WARNING: scripts are not allowed in tooltips');
+
+		return sOutput;
 	}
 };
 
@@ -731,9 +747,7 @@ const CombodoTooltip = {
 	 * @constructor
 	 */
 	InitTooltipFromMarkup: function (oElem, bForce = false) {
-		const oOptions = {
-			allowHTML: true,    // Always true so line breaks can work. Don't worry content will be sanitized.
-		};
+		const oOptions = {};
 
 		// First, check if the tooltip isn't already instantiated
 		if ((oElem.attr('data-tooltip-instantiated') === 'true') && (bForce === false)) {
@@ -746,24 +760,18 @@ const CombodoTooltip = {
 		// Content must be reworked before getting into the tooltip
 		// - Should we enable HTML content or keep text as is
 		const bEnableHTML = oElem.attr('data-tooltip-html-enabled') === 'true';
+		oOptions['allowHTML'] = bEnableHTML;
 
 		// - Content should be sanitized unless the developer says otherwise
 		// Note: Condition is inversed on purpose. When the developer is instantiating a tooltip,
-		// we want him/her to explicitly declare that he/she wants the sanitizer to be skipped.
+		// we want they to explicitly declare that they want the sanitizer to be skipped.
 		// Whereas in this code, it's easier to follow the logic with the variable oriented this way.
 		const bSanitizeContent = oElem.attr('data-tooltip-sanitizer-skipped') !== 'true';
 
-		// - Sanitize content and make sure line breaks are kept
-		const oTmpContentElem = $('<div />').html(oElem.attr('data-tooltip-content'));
-		let sContent = '';
-		if (bEnableHTML) {
-			sContent = oTmpContentElem.html();
-			if (bSanitizeContent) {
-				sContent = sContent.replace(/<script/g, '&lt;script WARNING: scripts are not allowed in tooltips');
-			}
-		} else {
-			sContent = oTmpContentElem.text();
-			sContent = sContent.replace(/(\r\n|\n\r|\r|\n)/g, '<br/>');
+		let sContent = oElem.attr('data-tooltip-content');
+		// - Check if both HTML and sanitizer are enabled
+		if (bEnableHTML && bSanitizeContent) {
+			sContent = CombodoGlobalToolbox.FilterXSS(sContent);
 		}
 		oOptions['content'] = sContent;