Browse Source

Fix for dol_string_onlythesehtmlattributes()

Laurent Destailleur 3 years ago
parent
commit
654cd8bd1c

+ 3 - 2
htdocs/admin/translation.php

@@ -43,6 +43,7 @@ $langcode = GETPOST('langcode', 'alphanohtml');
 $transkey = GETPOST('transkey', 'alphanohtml');
 $transvalue = GETPOST('transvalue', 'restricthtml');
 
+
 $mode = GETPOST('mode', 'aZ09') ? GETPOST('mode', 'aZ09') : 'searchkey';
 
 $limit = GETPOST('limit', 'int') ? GETPOST('limit', 'int') : $conf->liste_limit;
@@ -477,9 +478,9 @@ if ($mode == 'searchkey') {
 	print $formadmin->select_language($langcode, 'langcode', 0, null, 0, 0, 0, 'maxwidth250', 1);
 	print '</td>'."\n";
 	print '<td>';
-	print '<input type="text" class="flat maxwidthonsmartphone" name="transkey" value="'.$transkey.'">';
+	print '<input type="text" class="flat maxwidthonsmartphone" name="transkey" value="'.dol_escape_htmltag($transkey).'">';
 	print '</td><td>';
-	print '<input type="text" class="quatrevingtpercent" name="transvalue" value="'.$transvalue.'">';
+	print '<input type="text" class="quatrevingtpercent" name="transvalue" value="'.dol_escape_htmltag($transvalue).'">';
 	// Limit to superadmin
 	/*if (! empty($conf->multicompany->enabled) && !$user->entity)
 	{

+ 10 - 7
htdocs/core/lib/functions.lib.php

@@ -838,7 +838,7 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options =
 				// We should also exclude non expected HTML attributes and clean content of some attributes.
 				if (!empty($conf->global->MAIN_RESTRICTHTML_REMOVE_ALSO_BAD_ATTRIBUTES)) {
 					// Warning, the function may add a LF so we are forced to trim to compare with old $out without having always a difference and an infinit loop.
-					$out = trim(dol_string_onlythesehtmlattributes($out));
+					$out = dol_string_onlythesehtmlattributes($out);
 				}
 
 				// Restore entity &apos; into &#39; (restricthtml is for html content so we can use html entity)
@@ -6458,7 +6458,8 @@ function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1,
 
 /**
  *	Clean a string from some undesirable HTML tags.
- *  Note. Not as secured as dol_string_onlythesehtmltags().
+ *  Note: Complementary to dol_string_onlythesehtmltags().
+ *  This method is used for example when option MAIN_RESTRICTHTML_REMOVE_ALSO_BAD_ATTRIBUTES is set to 1.
  *
  *	@param	string	$stringtoclean			String to clean
  *  @param	array	$allowed_attributes		Array of tags not allowed
@@ -6469,10 +6470,11 @@ function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1,
 function dol_string_onlythesehtmlattributes($stringtoclean, $allowed_attributes = array("allow", "allowfullscreen", "alt", "class", "contenteditable", "data-html", "frameborder", "height", "href", "id", "name", "src", "style", "target", "title", "width"))
 {
 	if (class_exists('DOMDocument') && !empty($stringtoclean)) {
-		$stringtoclean = '<html><body>'.$stringtoclean.'</body></html>';
+		$stringtoclean = '<?xml encoding="UTF-8"><html><body>'.$stringtoclean.'</body></html>';
 
-		$dom = new DOMDocument();
+		$dom = new DOMDocument(null, 'UTF-8');
 		$dom->loadHTML($stringtoclean, LIBXML_ERR_NONE|LIBXML_HTML_NOIMPLIED|LIBXML_HTML_NODEFDTD|LIBXML_NONET|LIBXML_NOWARNING|LIBXML_NOXMLDECL);
+
 		if (is_object($dom)) {
 			for ($els = $dom->getElementsByTagname('*'), $i = $els->length - 1; $i >= 0; $i--) {
 				for ($attrs = $els->item($i)->attributes, $ii = $attrs->length - 1; $ii >= 0; $ii--) {
@@ -6505,9 +6507,10 @@ function dol_string_onlythesehtmlattributes($stringtoclean, $allowed_attributes
 		$return = $dom->saveHTML();
 		//$return = '<html><body>aaaa</p>bb<p>ssdd</p>'."\n<p>aaa</p>aa<p>bb</p>";
 
-		$return = preg_replace('/^<html><body>/', '', $return);
-		$return = preg_replace('/<\/body><\/html>$/', '', $return);
-		return $return;
+		$return = preg_replace('/^'.preg_quote('<?xml encoding="UTF-8">', '/').'/', '', $return);
+		$return = preg_replace('/^'.preg_quote('<html><body>', '/').'/', '', $return);
+		$return = preg_replace('/'.preg_quote('</body></html>', '/').'$/', '', $return);
+		return trim($return);
 	} else {
 		return $stringtoclean;
 	}

+ 3 - 2
htdocs/externalsite/admin/index.php

@@ -56,9 +56,10 @@ if ($action == 'update') {
 
 	$label  = GETPOST('EXTERNALSITE_LABEL', 'alphanohtml');
 
+	// exturl can be an url or a HTML string
 	$exturl = GETPOST('EXTERNALSITE_URL', 'none');
 	$exturl = dol_string_onlythesehtmltags($exturl, 1, 1, 0, 1);
-	$exturl = trim(dol_string_onlythesehtmlattributes($exturl));
+	$exturl = dol_string_onlythesehtmlattributes($exturl);
 
 	$i += dolibarr_set_const($db, 'EXTERNALSITE_LABEL', trim($label), 'chaine', 0, '', $conf->entity);
 	$i += dolibarr_set_const($db, 'EXTERNALSITE_URL', trim($exturl), 'chaine', 0, '', $conf->entity);
@@ -111,7 +112,7 @@ print '<td><textarea class="flat minwidth500" name="EXTERNALSITE_URL">';
 
 $exturl = GETPOST('EXTERNALSITE_URL', 'none');
 $exturl = dol_string_onlythesehtmltags($exturl, 1, 1, 0, 1);
-$exturl = trim(dol_string_onlythesehtmlattributes($exturl));
+$exturl = dol_string_onlythesehtmlattributes($exturl);
 
 print (GETPOSTISSET('EXTERNALSITE_URL') ? $exturl : (empty($conf->global->EXTERNALSITE_URL) ? '' : $conf->global->EXTERNALSITE_URL));
 print '</textarea></td>';

+ 1 - 1
htdocs/langs/en_US/bookmarks.lang

@@ -15,7 +15,7 @@ UrlOrLink=URL
 BehaviourOnClick=Behaviour when a bookmark URL is selected
 CreateBookmark=Create bookmark
 SetHereATitleForLink=Set a name for the bookmark
-UseAnExternalHttpLinkOrRelativeDolibarrLink=Use an external/absolute link (https://URL) or an internal/relative link (/DOLIBARR_ROOT/htdocs/...)
+UseAnExternalHttpLinkOrRelativeDolibarrLink=Use an external/absolute link (https://externalurl.com) or an internal/relative link (/mypage.php). You can also use phone like tel:0123456.
 ChooseIfANewWindowMustBeOpenedOnClickOnBookmark=Choose if the linked page should open in the current tab or a new tab
 BookmarksManagement=Bookmarks management
 BookmarksMenuShortCut=Ctrl + shift + m

+ 5 - 1
test/phpunit/SecurityTest.php

@@ -672,10 +672,14 @@ class SecurityTest extends PHPUnit\Framework\TestCase
 	 */
 	public function testDolStringOnlyTheseHtmlAttributes()
 	{
+		$stringtotest = 'eée';
+		$decodedstring = dol_string_onlythesehtmlattributes($stringtotest);
+		$this->assertEquals('e&eacute;e', $decodedstring, 'Function did not sanitize correclty with test 1');
+
 		$stringtotest = '<div onload="ee"><a href="123"><span class="abc">abc</span></a></div>';
 		$decodedstring = dol_string_onlythesehtmlattributes($stringtotest);
 		$decodedstring = preg_replace("/\n$/", "", $decodedstring);
-		$this->assertEquals('<div><a href="123"><span class="abc">abc</span></a></div>', $decodedstring, 'Function did not sanitize correclty with test 1');
+		$this->assertEquals('<div><a href="123"><span class="abc">abc</span></a></div>', $decodedstring, 'Function did not sanitize correclty with test 2');
 
 		return 0;
 	}