diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 3e6bcd9d5d1..4334aa9a12d 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -753,9 +753,9 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = $out = dol_string_nohtmltag($out, 0); // Remove also other dangerous string sequences // '"' is dangerous because param in url can close the href= or src= and add javascript functions. - // '../' is dangerous because it allows dir transversals + // '../' or '..\' is dangerous because it allows dir transversals // Note &, '&', '&'... is a simple char like '&' alone but there is no reason to accept such way to encode input data. - $out = str_ireplace(array('&', '&', '&', '"', '"', '"', '"', '"', '/', '/', '/', '../'), '', $out); + $out = str_ireplace(array('&', '&', '&', '"', '"', '"', '"', '"', '/', '/', '\', '\', '/', '../', '..\\'), '', $out); } while ($oldstringtoclean != $out); // keep lines feed } @@ -768,9 +768,9 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = // Remove html tags $out = dol_html_entity_decode($out, ENT_COMPAT | ENT_HTML5, 'UTF-8'); // '"' is dangerous because param in url can close the href= or src= and add javascript functions. - // '../' is dangerous because it allows dir transversals + // '../' or '..\' is dangerous because it allows dir transversals // Note &, '&', '&'... is a simple char like '&' alone but there is no reason to accept such way to encode input data. - $out = str_ireplace(array('&', '&', '&', '"', '"', '"', '"', '"', '/', '/', '/', '../'), '', $out); + $out = str_ireplace(array('&', '&', '&', '"', '"', '"', '"', '"', '/', '/', '\', '\', '/', '../', '..\\'), '', $out); } while ($oldstringtoclean != $out); } break; diff --git a/htdocs/document.php b/htdocs/document.php index 59501cc4701..046eb9b14bf 100644 --- a/htdocs/document.php +++ b/htdocs/document.php @@ -195,7 +195,8 @@ if (!in_array($type, array('text/x-javascript')) && !dolIsAllowedForPreview($ori } // Security: Delete string ../ into $original_file -$original_file = str_replace("../", "/", $original_file); +$original_file = str_replace('../', '/', $original_file); +$original_file = str_replace('..\\', '/', $original_file); // Find the subdirectory name as the reference $refname = basename(dirname($original_file)."/"); diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 87edca884e4..81eff830b49 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -349,7 +349,8 @@ class SecurityTest extends PHPUnit\Framework\TestCase $_POST["param1"]="333"; $_GET["param2"]='a/b#e(pr)qq-rr\cc'; $_GET["param3"]='"na/b#e(pr)qq-rr\cc'; // Same than param2 + " and n - $_GET["param4"]='../dir'; + $_GET["param4a"]='../../dir'; + $_GET["param4b"]='..\..\dirwindows'; $_GET["param5"]="a_1-b"; $_POST["param6"]="">assertEquals($result, 'na/b#e(pr)qq-rr\cc', 'Test on param3'); - $result=GETPOST("param4", 'alpha'); // Must return string sanitized from ../ + $result=GETPOST("param4a", 'alpha'); // Must return string sanitized from ../ print __METHOD__." result=".$result."\n"; $this->assertEquals($result, 'dir'); + $result=GETPOST("param4b", 'alpha'); // Must return string sanitized from ../ + print __METHOD__." result=".$result."\n"; + $this->assertEquals($result, 'dirwindows'); + // Test with aZ09 $result=GETPOST("param1", 'aZ09'); @@ -412,7 +417,11 @@ class SecurityTest extends PHPUnit\Framework\TestCase print __METHOD__." result=".$result."\n"; $this->assertEquals($result, ''); - $result=GETPOST("param4", 'aZ09'); // Must return '' as string contains car not in aZ09 definition + $result=GETPOST("param4a", 'aZ09'); // Must return '' as string contains car not in aZ09 definition + print __METHOD__." result=".$result."\n"; + $this->assertEquals('', $result); + + $result=GETPOST("param4b", 'aZ09'); // Must return '' as string contains car not in aZ09 definition print __METHOD__." result=".$result."\n"; $this->assertEquals('', $result);