From 654cd8bd1c25158ca93e491c2f2ba63303bc98ba Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Fri, 17 Dec 2021 12:01:25 +0100 Subject: [PATCH] Fix for dol_string_onlythesehtmlattributes() --- htdocs/admin/translation.php | 5 +++-- htdocs/core/lib/functions.lib.php | 17 ++++++++++------- htdocs/externalsite/admin/index.php | 5 +++-- htdocs/langs/en_US/bookmarks.lang | 2 +- test/phpunit/SecurityTest.php | 6 +++++- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/htdocs/admin/translation.php b/htdocs/admin/translation.php index 4f8f8e04012..12b2eec700d 100644 --- a/htdocs/admin/translation.php +++ b/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 ''."\n"; print ''; - print ''; + print ''; print ''; - print ''; + print ''; // Limit to superadmin /*if (! empty($conf->multicompany->enabled) && !$user->entity) { diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 94148722862..ab48705cbc1 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/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 ' into ' (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 = ''.$stringtoclean.''; + $stringtoclean = ''.$stringtoclean.''; - $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 = 'aaaa

bb

ssdd

'."\n

aaa

aa

bb

"; - $return = preg_replace('/^/', '', $return); - $return = preg_replace('/<\/body><\/html>$/', '', $return); - return $return; + $return = preg_replace('/^'.preg_quote('', '/').'/', '', $return); + $return = preg_replace('/^'.preg_quote('', '/').'/', '', $return); + $return = preg_replace('/'.preg_quote('', '/').'$/', '', $return); + return trim($return); } else { return $stringtoclean; } diff --git a/htdocs/externalsite/admin/index.php b/htdocs/externalsite/admin/index.php index d8dbb316593..ba23b88e68e 100644 --- a/htdocs/externalsite/admin/index.php +++ b/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 ''; diff --git a/htdocs/langs/en_US/bookmarks.lang b/htdocs/langs/en_US/bookmarks.lang index d2378963a21..be0f2f7e25d 100644 --- a/htdocs/langs/en_US/bookmarks.lang +++ b/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 diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index d1e67b04a37..6ca58e0d450 100644 --- a/test/phpunit/SecurityTest.php +++ b/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ée', $decodedstring, 'Function did not sanitize correclty with test 1'); + $stringtotest = '
abc
'; $decodedstring = dol_string_onlythesehtmlattributes($stringtotest); $decodedstring = preg_replace("/\n$/", "", $decodedstring); - $this->assertEquals('
abc
', $decodedstring, 'Function did not sanitize correclty with test 1'); + $this->assertEquals('
abc
', $decodedstring, 'Function did not sanitize correclty with test 2'); return 0; }