diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index c7468e4c7c6..72b7845fe8c 100644 --- a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -68,6 +68,8 @@ function testSqlAndScriptInject($val, $type) // We clean string because some hacks try to obfuscate evil strings by inserting non printable chars. Example: 'java(ascci09)scr(ascii00)ipt' is processed like 'javascript' (whatever is place of evil ascii char) // We should use dol_string_nounprintableascii but function is not yet loaded/available $val = preg_replace('/[\x00-\x1F\x7F]/u', '', $val); // /u operator makes UTF8 valid characters being ignored so are not included into the replace + // We clean html comments because some hacks try to obfuscate evil strings by inserting HTML comments. Example: onerror=alert(1) + $val = preg_replace('//', '', $val); $inj = 0; // For SQL Injection (only GET are used to be included into bad escaped SQL requests) diff --git a/test/phpunit/AllTests.php b/test/phpunit/AllTests.php index 09c075c1083..d07cc34a544 100644 --- a/test/phpunit/AllTests.php +++ b/test/phpunit/AllTests.php @@ -77,8 +77,8 @@ class AllTests $suite = new PHPUnit\Framework\TestSuite('PHPUnit Framework'); - //require_once dirname(__FILE__).'/CoreTest.php'; - //$suite->addTestSuite('CoreTest'); + require_once dirname(__FILE__).'/CoreTest.php'; + $suite->addTestSuite('CoreTest'); require_once dirname(__FILE__).'/AdminLibTest.php'; $suite->addTestSuite('AdminLibTest'); require_once dirname(__FILE__).'/CompanyLibTest.php'; diff --git a/test/phpunit/CoreTest.php b/test/phpunit/CoreTest.php index 87c43798a0f..3169c4063c0 100644 --- a/test/phpunit/CoreTest.php +++ b/test/phpunit/CoreTest.php @@ -239,78 +239,99 @@ class CoreTest extends PHPUnit\Framework\TestCase /** - * testSqlAndScriptInject + * testSqlAndScriptInjectWithPHPUnit * * @return void */ - public function testSqlAndScriptInject() + public function testSqlAndScriptInjectWithPHPUnit() { - global $dolibarr_main_prod; - - global $dolibarr_main_url_root; - global $dolibarr_main_data_root; - global $dolibarr_main_document_root; - global $dolibarr_main_data_root_alt; - global $dolibarr_main_document_root_alt; - global $dolibarr_main_db_host; - global $dolibarr_main_db_port; - global $dolibarr_main_db_type; - global $dolibarr_main_db_prefix; - - // This is code copied from main.inc.php !!!!!!!!!!!!!!! - // phpcs:disable PEAR.NamingConventions.ValidFunctionName.NotCamelCaps /** - * Security: SQL Injection and XSS Injection (scripts) protection (Filters on GET, POST, PHP_SELF). + * Security: WAF layer for SQL Injection and XSS Injection (scripts) protection (Filters on GET, POST, PHP_SELF). * - * @param string $val Value - * @param string $type 1=GET, 0=POST, 2=PHP_SELF - * @return int >0 if there is an injection + * @param string $val Value brut found int $_GET, $_POST or PHP_SELF + * @param string $type 1=GET, 0=POST, 2=PHP_SELF, 3=GET without sql reserved keywords (the less tolerant test) + * @return int >0 if there is an injection, 0 if none */ function testSqlAndScriptInject($val, $type) { - // phpcs:enable - $inj = 0; - // For SQL Injection (only GET and POST are used to be included into bad escaped SQL requests) - if ($type != 2) - { - $inj += preg_match('/delete\s+from/i', $val); - $inj += preg_match('/create\s+table/i', $val); - $inj += preg_match('/update.+set.+=/i', $val); - $inj += preg_match('/insert\s+into/i', $val); - $inj += preg_match('/select.+from/i', $val); - $inj += preg_match('/union.+select/i', $val); - $inj += preg_match('/into\s+(outfile|dumpfile)/i', $val); - $inj += preg_match('/(\.\.%2f)+/i', $val); - } - // For XSS Injection done by adding javascript with script - // This is all cases a browser consider text is javascript: - // When it found ' - $inj += preg_match('/onerror\s*=/i', $val); // onerror can be set on img or any html tag like - $inj += preg_match('/onfocus\s*=/i', $val); // onfocus can be set on input text html tag like - $inj += preg_match('/onload\s*=/i', $val); // onload can be set on svg tag or other tag like body - //$inj += preg_match('/on[A-Z][a-z]+\*=/', $val); // To lock event handlers onAbort(), ... - $inj += preg_match('/:|:|:/i', $val); // refused string ':' encoded (no reason to have it encoded) to lock 'javascript:...' - //if ($type == 1) - //{ - $inj += preg_match('/javascript:/i', $val); - $inj += preg_match('/vbscript:/i', $val); - //} - // For XSS Injection done by adding javascript closing html tags like with onmousemove, etc... (closing a src or href tag with not cleaned param) - if ($type == 1) $inj += preg_match('/"/i', $val); // We refused " in GET parameters value - if ($type == 2) $inj += preg_match('/[;"]/', $val); // PHP_SELF is a file system path. It can contains spaces. - return $inj; + // Decode string first + // So error=alert(1) + $val = preg_replace('//', '', $val); + + $inj = 0; + // For SQL Injection (only GET are used to be included into bad escaped SQL requests) + if ($type == 1 || $type == 3) + { + $inj += preg_match('/delete\s+from/i', $val); + $inj += preg_match('/create\s+table/i', $val); + $inj += preg_match('/insert\s+into/i', $val); + $inj += preg_match('/select\s+from/i', $val); + $inj += preg_match('/into\s+(outfile|dumpfile)/i', $val); + $inj += preg_match('/user\s*\(/i', $val); // avoid to use function user() that return current database login + $inj += preg_match('/information_schema/i', $val); // avoid to use request that read information_schema database + $inj += preg_match('/ + $inj += preg_match('/ondrag([a-z]*)\s*=/i', $val); // + $inj += preg_match('/ontouch([a-z]*)\s*=/i', $val); // + $inj += preg_match('/on(abort|afterprint|beforeprint|beforeunload|blur|canplay|canplaythrough|change|click|contextmenu|copy|cut)\s*=/i', $val); + $inj += preg_match('/on(dblclick|drop|durationchange|ended|error|focus|focusin|focusout|hashchange|input|invalid)\s*=/i', $val); + $inj += preg_match('/on(keydown|keypress|keyup|load|loadeddata|loadedmetadata|loadstart|offline|online|pagehide|pageshow)\s*=/i', $val); + $inj += preg_match('/on(paste|pause|play|playing|progress|ratechange|resize|reset|scroll|search|seeking|select|show|stalled|start|submit|suspend)\s*=/i', $val); + $inj += preg_match('/on(timeupdate|toggle|unload|volumechange|waiting)\s*=/i', $val); + //$inj += preg_match('/on[A-Z][a-z]+\*=/', $val); // To lock event handlers onAbort(), ... + $inj += preg_match('/:|:|:/i', $val); // refused string ':' encoded (no reason to have it encoded) to lock 'javascript:...' + $inj += preg_match('/javascript\s*:/i', $val); + $inj += preg_match('/vbscript\s*:/i', $val); + // For XSS Injection done by adding javascript closing html tags like with onmousemove, etc... (closing a src or href tag with not cleaned param) + if ($type == 1) { + $val = str_replace('enclosure="', 'enclosure=X', $val); // We accept enclosure=" + $inj += preg_match('/"/i', $val); // We refused " in GET parameters value. + } + if ($type == 2) $inj += preg_match('/[;"]/', $val); // PHP_SELF is a file system path. It can contains spaces. + return $inj; } + // Run tests // More on https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet @@ -404,5 +425,9 @@ class CoreTest extends PHPUnit\Framework\TestCase $test='Set.constructor`alert\x281\x29```'; $result=testSqlAndScriptInject($test, 0); $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject iii'); + + $test="onerror=alert(1)"; + $result=testSqlAndScriptInject($test, 0); + $this->assertGreaterThanOrEqual($expectedresult, $result, 'Error on testSqlAndScriptInject jjj'); } }