From 9b58b61d60cc412a56f746a91dd9a6ed106ed71a Mon Sep 17 00:00:00 2001
From: Laurent Destailleur <eldy@destailleur.fr>
Date: Wed, 10 Aug 2022 20:18:36 +0200
Subject: [PATCH] Enhance MAIN_RESTRICTHTML_REMOVE_ALSO_BAD_ATTRIBUTES
 #yogosha12008

---
 htdocs/core/lib/functions.lib.php | 13 +++++++++----
 test/phpunit/SecurityTest.php     | 15 +++++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php
index 90698c95f80..7edfd6d6689 100644
--- a/htdocs/core/lib/functions.lib.php
+++ b/htdocs/core/lib/functions.lib.php
@@ -6821,10 +6821,11 @@ function dol_string_onlythesehtmlattributes($stringtoclean, $allowed_attributes
 				for ($attrs = $els->item($i)->attributes, $ii = $attrs->length - 1; $ii >= 0; $ii--) {
 					//var_dump($attrs->item($ii));
 					if (! empty($attrs->item($ii)->name)) {
-						// Delete attribute if not into allowed_attributes
 						if (! in_array($attrs->item($ii)->name, $allowed_attributes)) {
+							// Delete attribute if not into allowed_attributes
 							$els->item($i)->removeAttribute($attrs->item($ii)->name);
 						} elseif (in_array($attrs->item($ii)->name, array('style'))) {
+							// If attribute is 'style'
 							$valuetoclean = $attrs->item($ii)->value;
 
 							if (isset($valuetoclean)) {
@@ -6833,10 +6834,14 @@ function dol_string_onlythesehtmlattributes($stringtoclean, $allowed_attributes
 									$valuetoclean = preg_replace('/\/\*.*\*\//m', '', $valuetoclean);	// clean css comments
 									$valuetoclean = preg_replace('/position\s*:\s*[a-z]+/mi', '', $valuetoclean);
 									if ($els->item($i)->tagName == 'a') {	// more paranoiac cleaning for clickable tags.
-										$valuetoclean = preg_replace('/display\s*://m', '', $valuetoclean);
-										$valuetoclean = preg_replace('/z-index\s*://m', '', $valuetoclean);
-										$valuetoclean = preg_replace('/\s+(top|left|right|bottom)\s*://m', '', $valuetoclean);
+										$valuetoclean = preg_replace('/display\s*:/mi', '', $valuetoclean);
+										$valuetoclean = preg_replace('/z-index\s*:/mi', '', $valuetoclean);
+										$valuetoclean = preg_replace('/\s+(top|left|right|bottom)\s*:/mi', '', $valuetoclean);
 									}
+
+									// We do not allow logout|passwordforgotten.php and action= into the content of a "style" tag
+									$valuetoclean = preg_replace('/(logout|passwordforgotten)\.php/mi', '', $valuetoclean);
+									$valuetoclean = preg_replace('/action=/mi', '', $valuetoclean);
 								} while ($oldvaluetoclean != $valuetoclean);
 							}
 
diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php
index 4f7a78790f1..0b77e613008 100644
--- a/test/phpunit/SecurityTest.php
+++ b/test/phpunit/SecurityTest.php
@@ -382,6 +382,9 @@ class SecurityTest extends PHPUnit\Framework\TestCase
 		$_POST["param13b"]='&#110; &#x6E; &gt; &lt; &quot; <a href=\"j&#x61vascript:alert(document.domain)\">XSS</a>';
 		$_POST["param14"]="Text with ' encoded with the numeric html entity converted into text entity &#39; (like when submited by CKEditor)";
 		$_POST["param15"]="<img onerror<=alert(document.domain)> src=>0xbeefed";
+		$_POST["param16"]='<a style="z-index: 1000">abc</a>';
+		$_POST["param17"]='<span style="background-image: url(logout.php)">abc</span>';
+		$_POST["param18"]='<span style="background-image: url(...?...action=aaa)">abc</span>';
 		//$_POST["param13"]='javascript%26colon%26%23x3B%3Balert(1)';
 		//$_POST["param14"]='javascripT&javascript#x3a alert(1)';
 
@@ -554,6 +557,18 @@ class SecurityTest extends PHPUnit\Framework\TestCase
 		print __METHOD__." result=".$result."\n";
 		$this->assertEquals('<img src="">0xbeefed', $result, 'Test 15b');
 
+		$result=GETPOST('param16', 'restricthtml');
+		print __METHOD__." result=".$result."\n";
+		$this->assertEquals('<a style=" 1000">abc</a>', $result, 'Test tag a with forbidden attribute z-index');
+
+		$result=GETPOST('param17', 'restricthtml');
+		print __METHOD__." result=".$result."\n";
+		$this->assertEquals('<span style="background-image: url()">abc</span>', $result, 'Test anytag with a forbidden value for attribute');
+
+		$result=GETPOST('param18', 'restricthtml');
+		print __METHOD__." result=".$result."\n";
+		$this->assertEquals('<span style="background-image: url(...?...aaa)">abc</span>', $result, 'Test anytag with a forbidden value for attribute');
+
 		unset($conf->global->MAIN_RESTRICTHTML_REMOVE_ALSO_BAD_ATTRIBUTES);