From 4965ce8768d9b9235c8dad002a6959e215461808 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sun, 14 Mar 2021 16:13:03 +0100 Subject: [PATCH] Fix method to sanitize an URL --- htdocs/core/lib/functions.lib.php | 26 +++++++++++++++++++++++--- test/phpunit/SecurityTest.php | 22 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index e6547c8c5f1..3fda792cbb4 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -1001,14 +1001,34 @@ function dol_sanitizePathName($str, $newstr = '_', $unaccent = 1) } /** - * Clean a string to use it as an URL + * Clean a string to use it as an URL (into a href or src attribute) * * @param string $stringtoclean String to clean + * @param int $type 0=Accept all Url, 1=Clean external Url (keep only relative Url) * @return string Escaped string. */ -function dol_sanitizeUrl($stringtoclean) +function dol_sanitizeUrl($stringtoclean, $type = 1) { - $stringtoclean = str_replace('javascript', '', $stringtoclean); + // 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 + $stringtoclean = preg_replace('/[\x00-\x1F\x7F]/u', '', $stringtoclean); // /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); + + $stringtoclean = str_replace('\\', '/', $stringtoclean); + $stringtoclean = str_replace(array(':', '@'), '', $stringtoclean); + + do { + $oldstringtoclean = $stringtoclean; + $stringtoclean = str_replace(array('javascript', 'vbscript'), '', $stringtoclean); + if ($type == 1) { + $stringtoclean = str_replace(array(':', '&colon', ';'), '', $stringtoclean); + } + } while ($oldstringtoclean != $stringtoclean); + + if ($type == 1) { + $stringtoclean = preg_replace(array('/^[a-z]*\/\/+/i'), '', $stringtoclean); + } return $stringtoclean; } diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 9aaf1349f14..88afc406018 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -661,6 +661,28 @@ class SecurityTest extends PHPUnit\Framework\TestCase return 0; } + /** + * testDolSanitizeUrl + * + * @return void + */ + public function testDolSanitizeUrl() + { + global $conf,$user,$langs,$db; + $conf=$this->savconf; + $user=$this->savuser; + $langs=$this->savlangs; + $db=$this->savdb; + + $test = 'javajavascriptscript&cjavascriptolon;alert(1)'; + $result=dol_sanitizeUrl($test); + $this->assertEquals('alert(1)', $result, 'Test on dol_sanitizeUrl'); + + $test = '/javas:cript/google.com'; + $result=dol_sanitizeUrl($test); + $this->assertEquals('google.com', $result, 'Test on dol_sanitizeUrl'); + } + /** * testDolSanitizeFileName *