CVE-2021-25955 (PHP, published 2021-08-15T21:15Z)
Description: value: In “Dolibarr ERP CRM”, WYSIWYG Editor module, v2.8.1 to v13.0.2 are affected by a stored XSS vulnerability that allows low privileged application users to store malicious scripts in the “Private Note” field at “/adherents/note.php?id=1” endpoint. These scripts are executed in a victim’s browser when they open the page containing the vulnerable field. In the worst case, the victim who inadvertently triggers the attack is a highly privileged administrator. The injected scripts can extract the Session ID, which can lead to full Account takeover of the admin and due to other vulnerability (Improper Access Control on Private notes) a low privileged user can update the private notes which could lead to privilege escalation.
Severity: LOW (CVSS v3 base score: 9)
Weakness: CWE-79 \u2014 Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')
The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
Repository: https://github.com/Dolibarr/dolibarr
Files changed:
htdocs/core/lib/functions.lib.php
htdocs/main.inc.php
test/phpunit/SecurityTest.php
Commit message: Enhance the sanitizing.

Vulnerable code:
				// We replace chars encoded with numeric HTML entities with real char (to avoid to have numeric entities used for obfuscation of injections)
				$out = preg_replace_callback('/&#(x?[0-9][0-9a-f]+);/i', 'realCharForNumericEntities', $out);
 * This function is required by testSqlAndScriptInject().
		$newstringnumentity = hexdec(preg_replace('/^x/i', '', $newstringnumentity));
	// The numeric value we don't want as entities
	return '&#'.$matches[1];
		$_GET["param7"]='"c:\this is a path~1\aaa&#110;" abc<bad>def</bad>';
		print __METHOD__." result=".$result."\n";
		$this->assertEquals('"c:\this is a path~1\aaan" abcdef', $result);
		$this->assertEquals('n n &gt; &lt; &quot; <a href=\"alert(document.domain)\">XSS</a>', $result, 'Test that HTML entities are decoded with restricthtml, but only for common alpha chars');
		$this->assertEquals('n n &gt; &lt; &quot; <a href=\"jvascript:alert(document.domain)\">XSS</a>', $result, 'Test that HTML entities are decoded with restricthtml, but only for common alpha chars');

Fixed code:
				// We replace chars from a/A to z/Z encoded with numeric HTML entities with the real char so we won't loose the chars at the next step.
				// No need to use a loop here, this step is not to sanitize (this is done at next step, this is to try to save chars, even if they are
				// using a non coventionnel way to be encoded, to not have them sanitized just after)
				$out = preg_replace_callback('/&#(x?[0-9][0-9a-f]+;?)/i', 'realCharForNumericEntities', $out);

				// Now we remove all remaining HTML entities staring with a number. We don't want such entities.
 * WARNING: This function is required by testSqlAndScriptInject() and the GETPOST 'restricthtml'. Regex calling must be similar.
	//print  '$newstringnumentity='.$newstringnumentity;
		$newstringnumentity = hexdec(preg_replace('/;$/', '', preg_replace('/^x/i', '', $newstringnumentity)));
	// The numeric value we don't want as entities because they encode ascii char, and why using html entities on ascii except for haking ?
	return '&#'.$matches[1];	// Value will be unchanged because regex was /&#(  )/
		$_GET["param7"]='"c:\this is a path~1\aaa&#110; &#x&#x31;&#x31;&#x30;;" abc<bad>def</bad>';
		print __METHOD__." result param7 = ".$result."\n";
		$this->assertEquals('"c:\this is a path~1\aaan &#x;;;;" abcdef', $result);
		$this->assertEquals('n n &gt; &lt; &quot; <a href=\"alert(document.domain)\">XSS</a>', $result, 'Test 13 that HTML entities are decoded with restricthtml, but only for common alpha chars');
		$this->assertEquals('n n &gt; &lt; &quot; <a href=\"alert(document.domain)\">XSS</a>', $result, 'Test 13b that HTML entities are decoded with restricthtml, but only for common alpha chars');