diff --git a/htdocs/install/repair.php b/htdocs/install/repair.php index 7bba5c48ee2..17277d488b0 100644 --- a/htdocs/install/repair.php +++ b/htdocs/install/repair.php @@ -1324,7 +1324,7 @@ if ($ok && GETPOST('force_utf8_on_tables', 'alpha')) { foreach ($arrayofforeignkey as $tmptable => $foreignkeyname) { if ($table[0] == $tmptable) { print ''; - $sqltmp = 'ALTER TABLE '.$table[0].' DROP FOREIGN KEY '.$foreignkeyname; + $sqltmp = 'ALTER TABLE '.$db->sanitize($table[0]).' DROP FOREIGN KEY '.$db->sanitize($foreignkeyname); print $sqltmp; if ($force_utf8_on_tables == 'confirmed') { $resqltmp = $db->query($sqltmp); @@ -1349,8 +1349,8 @@ if ($ok && GETPOST('force_utf8_on_tables', 'alpha')) { print ''; print $table[0]; - $sql1 = "ALTER TABLE ".$table[0]." ROW_FORMAT=dynamic"; - $sql2 = "ALTER TABLE ".$table[0]." CONVERT TO CHARACTER SET utf8 COLLATE utf8_unicode_ci"; + $sql1 = "ALTER TABLE ".$db->sanitize($table[0])." ROW_FORMAT=dynamic"; + $sql2 = "ALTER TABLE ".$db->sanitize($table[0])." CONVERT TO CHARACTER SET utf8 COLLATE utf8_unicode_ci"; print ''; print ''; if ($force_utf8_on_tables == 'confirmed') { @@ -1371,7 +1371,7 @@ if ($ok && GETPOST('force_utf8_on_tables', 'alpha')) { // Restore dropped foreign keys foreach ($foreignkeystorestore as $tmptable => $foreignkeyname) { - $stringtofindinline = 'ALTER TABLE .* ADD CONSTRAINT '.$foreignkeyname; + $stringtofindinline = 'ALTER TABLE .* ADD CONSTRAINT '.$db->sanitize($foreignkeyname); $fileforkeys = DOL_DOCUMENT_ROOT.'/install/mysql/tables/'.$tmptable.'.key.sql'; //print 'Search in '.$fileforkeys.' to get '.$stringtofindinline."
\n"; @@ -1448,7 +1448,7 @@ if ($ok && GETPOST('force_utf8mb4_on_tables', 'alpha')) { foreach ($arrayofforeignkey as $tmptable => $foreignkeyname) { if ($table[0] == $tmptable) { print ''; - $sqltmp = 'ALTER TABLE '.$table[0].' DROP FOREIGN KEY '.$foreignkeyname; + $sqltmp = 'ALTER TABLE '.$db->sanitize($table[0]).' DROP FOREIGN KEY '.$db->sanitize($foreignkeyname); print $sqltmp; if ($force_utf8mb4_on_tables == 'confirmed') { $resqltmp = $db->query($sqltmp); @@ -1473,8 +1473,8 @@ if ($ok && GETPOST('force_utf8mb4_on_tables', 'alpha')) { print ''; print $table[0]; - $sql1 = "ALTER TABLE ".$table[0]." ROW_FORMAT=dynamic"; - $sql2 = "ALTER TABLE ".$table[0]." CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci"; + $sql1 = "ALTER TABLE ".$db->sanitize($table[0])." ROW_FORMAT=dynamic"; + $sql2 = "ALTER TABLE ".$db->sanitize($table[0])." CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci"; print ''; print ''; if ($force_utf8mb4_on_tables == 'confirmed') { @@ -1495,7 +1495,7 @@ if ($ok && GETPOST('force_utf8mb4_on_tables', 'alpha')) { // Restore dropped foreign keys foreach ($foreignkeystorestore as $tmptable => $foreignkeyname) { - $stringtofindinline = 'ALTER TABLE .* ADD CONSTRAINT '.$foreignkeyname; + $stringtofindinline = 'ALTER TABLE .* ADD CONSTRAINT '.$db->sanitize($foreignkeyname); $fileforkeys = DOL_DOCUMENT_ROOT.'/install/mysql/tables/'.$tmptable.'.key.sql'; //print 'Search in '.$fileforkeys.' to get '.$stringtofindinline."
\n"; diff --git a/htdocs/public/partnership/new.php b/htdocs/public/partnership/new.php index 26500400ebc..c23db0bb4da 100644 --- a/htdocs/public/partnership/new.php +++ b/htdocs/public/partnership/new.php @@ -102,7 +102,7 @@ $user->loadDefaultValues(); */ function llxHeaderVierge($title, $head = "", $disablejs = 0, $disablehead = 0, $arrayofjs = [], $arrayofcss = []) { - global $user, $conf, $langs, $mysoc; + global $conf, $langs, $mysoc; top_htmlhead($head, $title, $disablejs, $disablehead, $arrayofjs, $arrayofcss); // Show html headers diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index b29c7299115..6dc4c94dfbc 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -233,7 +233,7 @@ class CodingPhpTest extends CommonClassTest } } - // Check we don't miss top_httphead() in any ajax pages + // Check we don't miss top_httphead() in any ajax pages if (preg_match('/ajax\//', $file['relativename'])) { //print "Analyze ajax page ".$file['relativename']."\n"; $ok = true; @@ -247,27 +247,27 @@ class CodingPhpTest extends CommonClassTest //exit; } - // Check for unauthorised vardumps + // Check for unauthorised vardumps if (!preg_match('/test\/phpunit/', $file['fullname'])) { $this->verifyNoActiveVardump($filecontent, $report_filepath); } - // Check get_class followed by __METHOD__ - $ok = true; - $matches = array(); - preg_match_all('/'.preg_quote('get_class($this)."::".__METHOD__', '/').'/', $filecontent, $matches, PREG_SET_ORDER); + // Check get_class followed by __METHOD__ + $ok = true; + $matches = array(); + preg_match_all('/'.preg_quote('get_class($this)."::".__METHOD__', '/').'/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { $ok = false; break; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found string get_class($this)."::".__METHOD__ that must be replaced with __METHOD__ only in '.$file['relativename']); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found string get_class($this)."::".__METHOD__ that must be replaced with __METHOD__ only in '.$file['relativename']); + //exit; - // Check string $this->db->idate without quotes - $ok = true; - $matches = array(); - preg_match_all('/(..)\s*\.\s*\$this->db->idate\(/', $filecontent, $matches, PREG_SET_ORDER); + // Check string $this->db->idate without quotes + $ok = true; + $matches = array(); + preg_match_all('/(..)\s*\.\s*\$this->db->idate\(/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if ($val[1] != '\'"' && $val[1] != '\'\'') { $ok = false; @@ -275,17 +275,17 @@ class CodingPhpTest extends CommonClassTest } //if ($reg[0] != 'db') $ok=false; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found a $this->db->idate to forge a sql request without quotes around this date field '.$file['relativename']); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found a $this->db->idate to forge a sql request without quotes around this date field '.$file['relativename']); + //exit; - // Check sql string DELETE|OR|AND|WHERE|INSERT ... yyy = ".$xxx - // with xxx that is not 'thi' (for $this->db->sanitize) and 'db-' (for $db->sanitize). It means we forget a ' if string, or an (int) if int, when forging sql request. - $ok = true; - $matches = array(); - preg_match_all('/(DELETE|OR|AND|WHERE|INSERT)\s.*([^\s][^\s][^\s])\s*=\s*(\'|")\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER); + // Check sql string DELETE|OR|AND|WHERE|INSERT ... yyy = ".$xxx + // with xxx that is not 'thi' (for $this->db->sanitize) and 'db-' (for $db->sanitize). It means we forget a ' if string, or an (int) if int, when forging sql request. + $ok = true; + $matches = array(); + preg_match_all('/(DELETE|OR|AND|WHERE|INSERT)\s.*([^\s][^\s][^\s])\s*=\s*(\'|")\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if ($val[2] == 'ity' && $val[4] == 'con') { // exclude entity = ".$conf->entity continue; @@ -300,14 +300,14 @@ class CodingPhpTest extends CommonClassTest $ok = false; break; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non quoted or not casted var in sql request '.$file['relativename'].' - Bad.'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non quoted or not casted var in sql request '.$file['relativename'].' - Bad.'); + //exit; - // Check that forged sql string is using ' instead of " as string PHP quotes - $ok = true; - $matches = array(); - preg_match_all('/\$sql \.= \'\s*VALUES.*\$/', $filecontent, $matches, PREG_SET_ORDER); + // Check that forged sql string is not using NOW() inside SQL + $ok = true; + $matches = array(); + preg_match_all('/NOW\(\)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { //if ($val[1] != '\'"' && $val[1] != '\'\'') { var_dump($matches); @@ -316,26 +316,42 @@ class CodingPhpTest extends CommonClassTest //} //if ($reg[0] != 'db') $ok=false; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables in file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELECT ".$myvar...'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found a forged SQL string that contains the function NOW() in file '.$file['relativename'].' Using this SQL function is forbidden. See https://wiki.dolibarr.org/index.php?title=Language_and_development_rules#SQL_Coding_rules'); + //exit; - // Check that forged sql string is using ' instead of " as string PHP quotes - $ok = true; - $matches = array(); - preg_match_all('/\$sql \.?= \'SELECT.*\$/', $filecontent, $matches, PREG_SET_ORDER); + // Check that forged sql string is using ' instead of " as string PHP quotes + $ok = true; + $matches = array(); + preg_match_all('/\$sql \.= \'\s*VALUES.*\$/', $filecontent, $matches, PREG_SET_ORDER); + foreach ($matches as $key => $val) { + //if ($val[1] != '\'"' && $val[1] != '\'\'') { + var_dump($matches); + $ok = false; + break; + //} + //if ($reg[0] != 'db') $ok=false; + } + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables in file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELECT ".$myvar...'); + //exit; + + // Check that forged sql string is using ' instead of " as string PHP quotes + $ok = true; + $matches = array(); + preg_match_all('/\$sql \.?= \'SELECT.*\$/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { var_dump($matches); $ok = false; break; } - $this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables in file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELECT ".$myvar...'); + $this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables in file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELECT ".$myvar...'); - // Check sql string VALUES ... , ".$xxx - // with xxx that is not 'db-' (for $db->escape). It means we forget a ' if string, or an (int) if int, when forging sql request. - $ok = true; - $matches = array(); - preg_match_all('/(VALUES).*,\s*"\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER); + // Check sql string VALUES ... , ".$xxx + // with xxx that is not 'db-' (for $db->escape). It means we forget a ' if string, or an (int) if int, when forging sql request. + $ok = true; + $matches = array(); + preg_match_all('/(VALUES).*,\s*"\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if ($val[1] == 'VALUES' && $val[2] == 'db-') { // exclude $db->escape( continue; @@ -347,30 +363,30 @@ class CodingPhpTest extends CommonClassTest $ok = false; break; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non quoted or not casted var in sql request '.$file['relativename'].' - Bad.'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non quoted or not casted var in sql request '.$file['relativename'].' - Bad.'); + //exit; - // Check '".$xxx non escaped + // Check '".$xxx non escaped - // Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request. - $ok = true; - $matches = array(); - preg_match_all('/=\s*\'"\s*\.\s*\$this->(....)/', $filecontent, $matches, PREG_SET_ORDER); + // Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request. + $ok = true; + $matches = array(); + preg_match_all('/=\s*\'"\s*\.\s*\$this->(....)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if ($val[1] != 'db->' && $val[1] != 'esca') { $ok = false; break; } } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 1) in '.$file['relativename'].' - Bad.'); + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 1) in '.$file['relativename'].' - Bad.'); - // Check string sql|set|WHERE|...'".$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request. - $ok = true; - $matches = array(); - $found = ""; - preg_match_all('/(sql|SET|WHERE|where|INSERT|insert|VALUES|LIKE).+\s*\'"\s*\.\s*\$(.......)/', $filecontent, $matches, PREG_SET_ORDER); + // Check string sql|set|WHERE|...'".$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request. + $ok = true; + $matches = array(); + $found = ""; + preg_match_all('/(sql|SET|WHERE|where|INSERT|insert|VALUES|LIKE).+\s*\'"\s*\.\s*\$(.......)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if (! in_array($val[2], array('this->d', 'this->e', 'db->esc', 'dbs->es', 'dbs->id', 'mydb->e', 'dbsessi', 'db->ida', 'escaped', 'exclude', 'include'))) { $found = $val[0]; @@ -379,15 +395,15 @@ class CodingPhpTest extends CommonClassTest } //if ($reg[0] != 'db') $ok=false; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 2) in '.$file['relativename'].': '.$found.' - Bad.'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 2) in '.$file['relativename'].': '.$found.' - Bad.'); + //exit; - // Check string sql|set...'.$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request. - $ok = true; - $matches = array(); - $found = ""; - preg_match_all('/(\$sql|SET\s|WHERE\s|INSERT\s|VALUES\s|VALUES\().+\s*\'\s*\.\s*\$(.........)/', $filecontent, $matches, PREG_SET_ORDER); + // Check string sql|set...'.$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request. + $ok = true; + $matches = array(); + $found = ""; + preg_match_all('/(\$sql|SET\s|WHERE\s|INSERT\s|VALUES\s|VALUES\().+\s*\'\s*\.\s*\$(.........)/', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if (! in_array($val[2], array('this->db-', 'db->prefi', 'db->sanit', 'dbs->pref', 'dbs->sani', 'conf->ent', 'key : \'\')', 'key])."\')', 'excludefi', 'regexstri', ''))) { $found = $val[0]; @@ -397,17 +413,17 @@ class CodingPhpTest extends CommonClassTest } //if ($reg[0] != 'db') $ok=false; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 3) in '.$file['relativename'].': '.$found.' - Bad.'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 3) in '.$file['relativename'].': '.$found.' - Bad.'); + //exit; - // Check string sql|set...=".... without (int). It means we forget a cast (int) when forging sql request. - $ok = true; - $matches = array(); - $found = ""; - // $sql .= " field = ".(isset($this->field) ? $this->escape($this->field) : "null")... is KO - // $sql .= " field = ".(isset($this->field) ? "'".$this->escape($this->field)."'" : "null")... is OK - /* + // Check string sql|set...=".... without (int). It means we forget a cast (int) when forging sql request. + $ok = true; + $matches = array(); + $found = ""; + // $sql .= " field = ".(isset($this->field) ? $this->escape($this->field) : "null")... is KO + // $sql .= " field = ".(isset($this->field) ? "'".$this->escape($this->field)."'" : "null")... is OK + /* preg_match_all('/(\$sql|VALUES\()[^\'\n]*[^\'\n]"\s*\.\s*([^\n]+)\n/m', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { if (! preg_match('/^(implode\(\' OR \', \$search|implode\(\' AND \', \$search|MAIN_DB_PREFIX|accountancy_code|\w+::|\$key|\$db->prefix|\$this->db->prefix|\$predefinedgroupwhere|\$db->sanitize|\$this->db->sanitize|\$db->ifsql|\$db->decrypt|\(int\)|\(float\)|\(\(int\)|\(\(float\)|\$conf->entity|getEntity|\$this->from)/', $val[2])) { @@ -445,14 +461,14 @@ class CodingPhpTest extends CommonClassTest } //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; $this->assertTrue($ok, 'Found non escaped or non casted string in building of a sql request (case 4) in '.$file['relativename'].': '.$found.' - Bad.'); - */ + */ - // Checks with IN + // Checks with IN - // Check string ' IN (".xxx' or ' IN (\'.xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. - $ok = true; - $matches = array(); - preg_match_all('/\s+IN\s*\([\'"]\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER); + // Check string ' IN (".xxx' or ' IN (\'.xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. + $ok = true; + $matches = array(); + preg_match_all('/\s+IN\s*\([\'"]\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { //var_dump($val); if (!in_array($val[1], array('$db->sani', '$this->db', 'getEntity', 'WON\',\'L', 'self::STA', 'Commande:', 'CommandeF', 'Entrepot:', 'Facture::', 'FactureFo', 'ExpenseRe', 'Societe::', 'Ticket::S'))) { @@ -461,14 +477,14 @@ class CodingPhpTest extends CommonClassTest } //if ($reg[0] != 'db') $ok=false; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non sanitized string in building of a IN or NOT IN sql request '.$file['relativename'].' - Bad.'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non sanitized string in building of a IN or NOT IN sql request '.$file['relativename'].' - Bad.'); + //exit; - // Check string ' IN (\'".xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. - $ok = true; - $matches = array(); - preg_match_all('/\s+IN\s*\(\'"\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER); + // Check string ' IN (\'".xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request. + $ok = true; + $matches = array(); + preg_match_all('/\s+IN\s*\(\'"\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) { //var_dump($val); if (!in_array($val[1], array('$db->sani', '$this->db', 'getEntity', 'WON\',\'L', 'self::STA', 'Commande:', 'CommandeF', 'Entrepot:', 'Facture::', 'FactureFo', 'ExpenseRe', 'Societe::', 'Ticket::S'))) { @@ -477,9 +493,9 @@ class CodingPhpTest extends CommonClassTest } //if ($reg[0] != 'db') $ok=false; } - //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; - $this->assertTrue($ok, 'Found non sanitized string in building of a IN or NOT IN sql request '.$file['relativename'].' - Bad.'); - //exit; + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + $this->assertTrue($ok, 'Found non sanitized string in building of a IN or NOT IN sql request '.$file['relativename'].' - Bad.'); + //exit; // Test that output of $_SERVER\[\'QUERY_STRING\'\] is escaped. $ok = true; @@ -765,6 +781,7 @@ class CodingPhpTest extends CommonClassTest private function verifyIsModuleEnabledOk(&$filecontent, $filename) { // Verify that only known modules are used + $matches = array(); preg_match_all("/isModEnabled\\(\s*[\"']([^\$\"']+)[\"']\\s*\\)/", $filecontent, $matches, PREG_SET_ORDER); foreach ($matches as $key => $val) {