diff --git a/htdocs/accountancy/admin/accountmodel.php b/htdocs/accountancy/admin/accountmodel.php index 80dcd2b1807f441bd07e0565063ac511be57979d..736338d1b88e88346b2ee840507c9679a687a012 100644 --- a/htdocs/accountancy/admin/accountmodel.php +++ b/htdocs/accountancy/admin/accountmodel.php @@ -599,7 +599,7 @@ if ($id) $fieldlist=explode(',',$tabfield[$id]); - print '<form action="'.dol_escape_htmltag($_SERVER['PHP_SELF']).'?id='.$id.'" method="POST">'; + print '<form action="'.$_SERVER['PHP_SELF'].'?id='.$id.'" method="POST">'; print '<input type="hidden" name="token" value="'.$_SESSION['newtoken'].'">'; print '<table class="noborder" width="100%">'; diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php index 407a0dbb1d0ebab33591471086d29b416382e06d..022aa4d5e4461765daee7f276b4875c002f95108 100644 --- a/htdocs/main.inc.php +++ b/htdocs/main.inc.php @@ -99,7 +99,7 @@ function test_sql_and_script_inject($val, $type) $sql_inj += preg_match('/base[\s]+href/si', $val); $sql_inj += preg_match('/<.*onmouse/si', $val); // onmousexxx can be set on img or any html tag like <img title='...' onmouseover=alert(1)> $sql_inj += preg_match('/onerror\s*=/i', $val); // onerror can be set on img or any html tag like <img title='...' onerror = alert(1)> -// $sql_inj += preg_match('/onfocus\s*=/i', $val); // onfocus can be set on input text html tag like <input type='text' value='...' onfocus = alert(1)> + $sql_inj += preg_match('/onfocus\s*=/i', $val); // onfocus can be set on input text html tag like <input type='text' value='...' onfocus = alert(1)> if ($type == 1) { $sql_inj += preg_match('/javascript:/i', $val); diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 581d5cdda89c06333d7c0269ace828b7aff2b31b..5df31da812cc9eabddb9f7cafa30a30e1bd068e7 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -172,10 +172,11 @@ class CodingPhpTest extends PHPUnit_Framework_TestCase $this->assertTrue($ok, 'Found non escaped string in building of a sql request '.$file['fullname'].' ('.$val[0].'). Bad.'); //exit; + // Test that output of $_SERVER\[\'QUERY_STRING\'\] is escaped. $ok=true; $matches=array(); // Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request. - preg_match_all('/(...................)\$_SERVER\[\'PHP_SELF\'\]/', $filecontent, $matches, PREG_SET_ORDER); + preg_match_all('/(...................)\$_SERVER\[\'QUERY_STRING\'\]/', $filecontent, $matches, PREG_SET_ORDER); foreach($matches as $key => $val) { if ($val[1] != 'dol_escape_htmltag(') @@ -185,6 +186,22 @@ class CodingPhpTest extends PHPUnit_Framework_TestCase } } $this->assertTrue($ok, 'Found a $_SERVER[\'QUERY_STRING\'] without dol_escape_htmltag around in file '.$file['fullname'].' ('.$val[1].'$_SERVER[\'QUERY_STRING\']). Bad.'); + + // Test that output of $_SERVER\[\'PHP_SELF\'\] is escaped (not done for the moment, did not found a way to forge value of $_SERVER['PHP_SELF'] by extern access). + /*$ok=true; + $matches=array(); + // Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request. + preg_match_all('/(...................)\$_SERVER\[\'PHP_SELF\'\]/', $filecontent, $matches, PREG_SET_ORDER); + foreach($matches as $key => $val) + { + if ($val[1] != 'dol_escape_htmltag(') + { + $ok=false; + break; + } + } + $this->assertTrue($ok, 'Found a $_SERVER[\'PHP_SELF\'] without dol_escape_htmltag around in file '.$file['fullname'].' ('.$val[1].'$_SERVER[\'PHP_SELF\']). Bad.'); + */ } return;