From 8ac368ce175a7add44aa0035d02769208f05ae65 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sat, 27 Jul 2024 18:07:37 +0200 Subject: [PATCH] FIX Better sanitizing for javascript. Fix <> bypass. --- htdocs/core/lib/functions.lib.php | 31 ++++++--- htdocs/main.inc.php | 8 ++- test/phpunit/ExampleTest.php | 102 ++++++++++++++++++++++++++++++ test/phpunit/SecurityTest.php | 63 +++++++++++++++--- 4 files changed, 183 insertions(+), 21 deletions(-) create mode 100644 test/phpunit/ExampleTest.php diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 031a87ac052..cdc7057c05d 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -7859,14 +7859,14 @@ function dol_string_nohtmltag($stringtoclean, $removelinefeed = 1, $pagecodeto = * Clean a string to keep only desirable HTML tags. * WARNING: This also clean HTML comments (because they can be used to obfuscate tag name). * - * @param string $stringtoclean String to clean - * @param int $cleanalsosomestyles Remove absolute/fixed positioning from inline styles - * @param int $removeclassattribute 1=Remove the class attribute from tags - * @param int $cleanalsojavascript Remove also occurrence of 'javascript:'. - * @param int $allowiframe Allow iframe tags. - * @param string[] $allowed_tags List of allowed tags to replace the default list - * @param int $allowlink Allow "link" tags. - * @return string String cleaned + * @param string $stringtoclean String to clean + * @param int $cleanalsosomestyles Remove absolute/fixed positioning from inline styles + * @param int $removeclassattribute 1=Remove the class attribute from tags + * @param int $cleanalsojavascript Remove also occurrence of 'javascript:'. + * @param int $allowiframe Allow iframe tags. + * @param string[] $allowed_tags List of allowed tags to replace the default list + * @param int $allowlink Allow "link" tags. + * @return string String cleaned * * @see dol_htmlwithnojs() dol_escape_htmltag() strip_tags() dol_string_nohtmltag() dol_string_neverthesehtmltags() */ @@ -7904,9 +7904,10 @@ function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1, $stringtoclean = preg_replace('/:/i', ':', $stringtoclean); $stringtoclean = preg_replace('/:|�+58|:/i', '', $stringtoclean); // refused string ':' encoded (no reason to have a : encoded like this) to disable 'javascript:...' + // Remove all HTML tags $temp = strip_tags($stringtoclean, $allowed_tags_string); // Warning: This remove also undesired , so may changes string obfuscated with that pass the injection detection into a harmfull string - if ($cleanalsosomestyles) { // Clean for remaining html tags + if ($cleanalsosomestyles) { // Clean for remaining html tags $temp = preg_replace('/position\s*:\s*(absolute|fixed)\s*!\s*important/i', '', $temp); // Note: If hacker try to introduce css comment into string to bypass this regex, the string must also be encoded by the dol_htmlentitiesbr during output so it become harmless } if ($removeclassattribute) { // Clean for remaining html tags @@ -8159,6 +8160,7 @@ function dol_htmlwithnojs($stringtoencode, $nouseofiframesandbox = 0, $check = ' } else { $out = $stringtoencode; + // First clean HTML content do { $oldstringtoclean = $out; @@ -8270,6 +8272,17 @@ function dol_htmlwithnojs($stringtoencode, $nouseofiframesandbox = 0, $check = ' // Restore entity ' into ' (restricthtml is for html content so we can use html entity) $out = preg_replace('/'/i', "'", $out); + + // Now remove js + // List of dom events is on https://www.w3schools.com/jsref/dom_obj_event.asp and https://developer.mozilla.org/en-US/docs/Web/Events + $out = preg_replace('/on(mouse|drag|key|load|touch|pointer|select|transition)[a-z]*\s*=/i', '', $out); // onmousexxx can be set on img or any html tag like + $out = preg_replace('/on(abort|after|animation|auxclick|before|blur|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', '', $out); + $out = preg_replace('/on(dblclick|drop|durationchange|emptied|end|ended|error|focus|focusin|focusout|formdata|gotpointercapture|hashchange|input|invalid)[a-z]*\s*=/i', '', $out); + $out = preg_replace('/on(lostpointercapture|offline|online|pagehide|pageshow)[a-z]*\s*=/i', '', $out); + $out = preg_replace('/on(paste|pause|play|playing|progress|ratechange|reset|resize|scroll|search|seeked|seeking|show|stalled|start|submit|suspend)[a-z]*\s*=/i', '', $out); + $out = preg_replace('/on(timeupdate|toggle|unload|volumechange|waiting|wheel)[a-z]*\s*=/i', '', $out); + // More not into the previous list + $out = preg_replace('/on(repeat|begin|finish|beforeinput)[a-z]*\s*=/i', '', $out); } while ($oldstringtoclean != $out); // Check the limit of external links that are automatically executed in a Rich text content. We count: diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index 30909d0cd59..91758cb05cd 100644 --- a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -211,6 +211,7 @@ function testSqlAndScriptInject($val, $type) } $inj += preg_match('/base\s+href/si', $val); $inj += preg_match('/=data:/si', $val); + // List of dom events is on https://www.w3schools.com/jsref/dom_obj_event.asp and https://developer.mozilla.org/en-US/docs/Web/Events $inj += preg_match('/on(mouse|drag|key|load|touch|pointer|select|transition)[a-z]*\s*=/i', $val); // onmousexxx can be set on img or any html tag like $inj += preg_match('/on(abort|after|animation|auxclick|before|blur|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', $val); @@ -219,11 +220,12 @@ function testSqlAndScriptInject($val, $type) $inj += preg_match('/on(paste|pause|play|playing|progress|ratechange|reset|resize|scroll|search|seeked|seeking|show|stalled|start|submit|suspend)[a-z]*\s*=/i', $val); $inj += preg_match('/on(timeupdate|toggle|unload|volumechange|waiting|wheel)[a-z]*\s*=/i', $val); // More not into the previous list - $inj += preg_match('/on(repeat|begin|finish|beforeinput)[a-z]*\s*=/i', $val); - // We refuse html into html because some hacks try to obfuscate evil strings by inserting HTML into HTML. Example: error=alert(1) to bypass test on onerror - $tmpval = preg_replace('/<[^<]+>/', '', $val); + // We refuse html into html because some hacks try to obfuscate evil strings by inserting HTML into HTML. + // Example: error=alert(1) or =alert(1) to bypass test on onerror= + $tmpval = preg_replace('/<[^<]*>/', '', $val); + // List of dom events is on https://www.w3schools.com/jsref/dom_obj_event.asp and https://developer.mozilla.org/en-US/docs/Web/Events $inj += preg_match('/on(mouse|drag|key|load|touch|pointer|select|transition)[a-z]*\s*=/i', $tmpval); // onmousexxx can be set on img or any html tag like $inj += preg_match('/on(abort|after|animation|auxclick|before|blur|cancel|canplay|canplaythrough|change|click|close|contextmenu|cuechange|copy|cut)[a-z]*\s*=/i', $tmpval); diff --git a/test/phpunit/ExampleTest.php b/test/phpunit/ExampleTest.php new file mode 100644 index 00000000000..eea00867997 --- /dev/null +++ b/test/phpunit/ExampleTest.php @@ -0,0 +1,102 @@ + + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * or see https://www.gnu.org/ + */ + +/** + * \file test/phpunit/ExampleTest.php + * \ingroup test + * \brief PHPUnit test to use as example or test. this one is not called by AllTest.php + * \remarks To run this script as CLI: phpunit filename.php + */ + +global $conf,$user,$langs,$db; +//define('TEST_DB_FORCE_TYPE','mysql'); // This is to force using mysql driver +//require_once 'PHPUnit/Autoload.php'; + +if (! defined('NOREQUIRESOC')) { + define('NOREQUIRESOC', '1'); +} +if (! defined('NOCSRFCHECK')) { + define('NOCSRFCHECK', '1'); +} +if (! defined('NOTOKENRENEWAL')) { + define('NOTOKENRENEWAL', '1'); +} +if (! defined('NOREQUIREMENU')) { + define('NOREQUIREMENU', '1'); // If there is no menu to show +} +if (! defined('NOREQUIREHTML')) { + define('NOREQUIREHTML', '1'); // If we don't need to load the html.form.class.php +} +if (! defined('NOREQUIREAJAX')) { + define('NOREQUIREAJAX', '1'); +} +if (! defined("NOLOGIN")) { + define("NOLOGIN", '1'); // If this page is public (can be called outside logged session) +} +if (! defined("NOSESSION")) { + define("NOSESSION", '1'); +} + +require_once dirname(__FILE__).'/../../htdocs/main.inc.php'; // We force include of main.inc.php instead of master.inc.php even if we are in CLI mode because it contains a lot of security components we want to test. +require_once dirname(__FILE__).'/../../htdocs/core/lib/security.lib.php'; +require_once dirname(__FILE__).'/../../htdocs/core/lib/security2.lib.php'; +require_once dirname(__FILE__).'/CommonClassTest.class.php'; + +if (empty($user->id)) { + print "Load permissions for admin user nb 1\n"; + $user->fetch(1); + $user->getrights(); +} +$conf->global->MAIN_DISABLE_ALL_MAILS = 1; + + +/** + * Class for PHPUnit tests + * + * @backupGlobals disabled + * @backupStaticAttributes enabled + * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. + */ +class SecurityTest extends CommonClassTest +{ + /** + * testExample + * + * @return string + */ + public function testExample() + { + global $conf,$user,$langs,$db; + $conf = $this->savconf; + $user = $this->savuser; + $langs = $this->savlangs; + $db = $this->savdb; + + // Force default mode + $conf->global->MAIN_RESTRICTHTML_ONLY_VALID_HTML = 0; + $conf->global->MAIN_RESTRICTHTML_ONLY_VALID_HTML_TIDY = 0; + $conf->global->MAIN_RESTRICTHTML_REMOVE_ALSO_BAD_ATTRIBUTES = 0; + $conf->global->MAIN_DISALLOW_URL_INTO_DESCRIPTIONS = 0; + + /* + $result = testSqlAndScriptInject('=alert(document.domain)', 0); + print __METHOD__." result=".$result."\n"; + $this->assertEquals(1, $result, 'Test example a'); + */ + } +} diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 89864c36451..368d95fff57 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -280,6 +280,14 @@ class SecurityTest extends CommonClassTest $result = testSqlAndScriptInject($test, 2); //print "test=".$test." result=".$result."\n"; $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject with a non valid UTF8 char'); + + $test = '=alert(document.domain)'; + $result = testSqlAndScriptInject($test, 0); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject with an obfuscated string that bypass the WAF'); + + $test = '=alert(document.domain)'; + $result = testSqlAndScriptInject($test, 0); + $this->assertEquals($expectedresult, $result, 'Error on testSqlAndScriptInject with an obfuscated string that bypass the WAF'); } /** @@ -328,8 +336,8 @@ class SecurityTest extends CommonClassTest $_POST["param13b"] = 'n n > < " XSS'; $_POST["param13c"] = 'aaa:<:bbb'; $_POST["param14"] = "Text with ' encoded with the numeric html entity converted into text entity ' (like when submitted by CKEditor)"; - $_POST["param15"] = " src=>0xbeefed"; - //$_POST["param15b"]="Example HTML

This is a paragraph.