CVE-2017-17899: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: SQL injection vulnerability in adherents/subscription/info.php in Dolibarr ERP/CRM version 6.0.4 allows remote attackers to execute arbitrary SQL commands via the rowid parameter.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 9.8
Published: 2017-12-27T17:08Z
Repository: https://github.com/Dolibarr/dolibarr
Language: PHP
Files changed: htdocs/adherents/subscription/info.php, htdocs/cashdesk/tpl/facturation1.tpl.php, htdocs/cashdesk/tpl/liste_articles.tpl.php, htdocs/cashdesk/tpl/menu.tpl.php, htdocs/cashdesk/tpl/ticket.tpl.php, htdocs/cashdesk/tpl/validation1.tpl.php, htdocs/cashdesk/tpl/validation2.tpl.php, htdocs/comm/multiprix.php, htdocs/fourn/index.php, htdocs/main.inc.php
Commit date: 2017-12-21 12:52:19 +0100
Commit message: FIX security vulnerability reported by ADLab of Venustech
Diff: diff --git a/htdocs/adherents/subscription/info.php b/htdocs/adherents/subscription/info.php
index c83771d22ff..05856c35f75 100644
--- a/htdocs/adherents/subscription/info.php
+++ b/htdocs/adherents/subscription/info.php
@@ -34,22 +34,22 @@ $langs->load("members");
 $langs->load("users");
 
 if (!$user->rights->adherent->lire)
 	accessforbidden();
 
-$rowid=isset($_GET["rowid"])?$_GET["rowid"]:$_POST["rowid"];
+$rowid=GETPOST("rowid",'int');
 
 
 
 /*
  * View
  */
 
-llxHeader();
-
 $form = new Form($db);
 
+llxHeader();
+
 $object = new Subscription($db);
 $result = $object->fetch($rowid);
 
 $head = subscription_prepare_head($object);
 
diff --git a/htdocs/cashdesk/tpl/facturation1.tpl.php b/htdocs/cashdesk/tpl/facturation1.tpl.php
index 580012847f5..4372f422518 100644
--- a/htdocs/cashdesk/tpl/facturation1.tpl.php
+++ b/htdocs/cashdesk/tpl/facturation1.tpl.php
@@ -17,10 +17,18 @@
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
 
+// Protection to avoid direct call of template
+if (empty($langs) || ! is_object($langs))
+{
+	print "Error, template page can't be called as URL";
+	exit;
+}
+
+
 $langs->load("main");
 $langs->load("bills");
 $langs->load("cashdesk");
 
 // Object $form must de defined
diff --git a/htdocs/cashdesk/tpl/liste_articles.tpl.php b/htdocs/cashdesk/tpl/liste_articles.tpl.php
index e4b6bc14436..65f48b0603c 100644
--- a/htdocs/cashdesk/tpl/liste_articles.tpl.php
+++ b/htdocs/cashdesk/tpl/liste_articles.tpl.php
@@ -15,10 +15,18 @@
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
 
+// Protection to avoid direct call of template
+if (empty($langs) || ! is_object($langs))
+{
+	print "Error, template page can't be called as URL";
+	exit;
+}
+
+
 require_once DOL_DOCUMENT_ROOT.'/societe/class/societe.class.php';
 require_once DOL_DOCUMENT_ROOT.'/product/class/product.class.php';
 
 $langs->load("main");
 $langs->load("bills");
diff --git a/htdocs/cashdesk/tpl/menu.tpl.php b/htdocs/cashdesk/tpl/menu.tpl.php
index fd9bcbca77c..4197db6b1b0 100644
--- a/htdocs/cashdesk/tpl/menu.tpl.php
+++ b/htdocs/cashdesk/tpl/menu.tpl.php
@@ -17,10 +17,18 @@
  *
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+// Protection to avoid direct call of template
+if (empty($langs) || ! is_object($langs))
+{
+	print "Error, template page can't be called as URL";
+	exit;
+}
+
+
 include_once DOL_DOCUMENT_ROOT.'/societe/class/societe.class.php';
 include_once DOL_DOCUMENT_ROOT.'/compta/bank/class/account.class.php';
 include_once DOL_DOCUMENT_ROOT.'/product/stock/class/entrepot.class.php';
 
 /*if (!empty($_SESSION["CASHDESK_ID_THIRDPARTY"]))
diff --git a/htdocs/cashdesk/tpl/ticket.tpl.php b/htdocs/cashdesk/tpl/ticket.tpl.php
index 82b84ba3b56..3937c9dbde3 100644
--- a/htdocs/cashdesk/tpl/ticket.tpl.php
+++ b/htdocs/cashdesk/tpl/ticket.tpl.php
@@ -14,10 +14,19 @@
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
+
+// Protection to avoid direct call of template
+if (empty($langs) || ! is_object($langs))
+{
+	print "Error, template page can't be called as URL";
+	exit;
+}
+
+
 include_once DOL_DOCUMENT_ROOT.'/compta/facture/class/facture.class.php';
 
 $langs->load("main");
 $langs->load('cashdesk');
 
diff --git a/htdocs/cashdesk/tpl/validation1.tpl.php b/htdocs/cashdesk/tpl/validation1.tpl.php
index bb3d63a9e58..2a390b99c65 100644
--- a/htdocs/cashdesk/tpl/validation1.tpl.php
+++ b/htdocs/cashdesk/tpl/validation1.tpl.php
@@ -14,10 +14,18 @@
  *
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+// Protection to avoid direct call of template
+if (empty($langs) || ! is_object($langs))
+{
+	print "Error, template page can't be called as URL";
+	exit;
+}
+
+
 $langs->load("main");
 $langs->load("bills");
 $langs->load("banks");
 
 // Object $form must de defined
diff --git a/htdocs/cashdesk/tpl/validation2.tpl.php b/htdocs/cashdesk/tpl/validation2.tpl.php
index a6e2fe2a3f3..a9889451bda 100644
--- a/htdocs/cashdesk/tpl/validation2.tpl.php
+++ b/htdocs/cashdesk/tpl/validation2.tpl.php
@@ -15,10 +15,18 @@
  * You should have received a copy of the GNU General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
 
+// Protection to avoid direct call of template
+if (empty($langs) || ! is_object($langs))
+{
+	print "Error, template page can't be called as URL";
+	exit;
+}
+
+
 $langs->load("main");
 $langs->load("bills");
 
 ?>
 
diff --git a/htdocs/comm/multiprix.php b/htdocs/comm/multiprix.php
index 80c6305fcec..dd4e6cef1fa 100644
--- a/htdocs/comm/multiprix.php
+++ b/htdocs/comm/multiprix.php
@@ -28,11 +28,12 @@ require_once DOL_DOCUMENT_ROOT.'/core/lib/company.lib.php';
 require_once DOL_DOCUMENT_ROOT.'/contact/class/contact.class.php';
 
 $langs->load("orders");
 $langs->load("companies");
 
-$_socid = $_GET["id"];
+$id = GETPOST('id','int');
+$_socid = GETPOST("id",'int');
 // Security check
 if ($user->societe_id > 0)
 {
 	$_socid = $user->societe_id;
 }
@@ -43,14 +44,14 @@ if ($user->societe_id > 0)
  */
 
 if ($_POST["action"] == 'setpricelevel')
 {
 	$soc = New Societe($db);
-	$soc->fetch($_GET["id"]);
+	$soc->fetch($id);
 	$soc->set_price_level($_POST["price_level"],$user);
 
-	header("Location: multiprix.php?id=".$_GET["id"]);
+	header("Location: multiprix.php?id=".$id);
 	exit;
 }
 
 
 /*
@@ -85,11 +86,11 @@ if ($_socid > 0)
 	if ($objsoc->client == 2) $tabchoice='prospect';
 
 	print '<form method="POST" action="multiprix.php?id='.$objsoc->id.'">';
 	print '<input type="hidden" name="token" value="'.$_SESSION['newtoken'].'">';
 	print '<input type="hidden" name="action" value="setpricelevel">';
-	
+
 	dol_fiche_head($head, $tabchoice, $langs->trans("ThirdParty"), 0, 'company');
 
 	print '<table width="100%" border="0">';
 	print '<tr><td class="tdtop">';
 	print '<table class="border" width="100%">';
@@ -120,16 +121,16 @@ if ($_socid > 0)
 
 	print "</td></tr>";
 	print "</table>";
 
 	dol_fiche_end();
-	
+
 	print '<div align="center"><input type="submit" class="button" value="'.$langs->trans("Save").'"></div>';
 
 	print "</form>";
 
-	
+
 	print '<br><br>';
 
 
 	/*
 	 * List historic of multiprices
diff --git a/htdocs/fourn/index.php b/htdocs/fourn/index.php
index 690835a5e99..8f34077ab34 100644
--- a/htdocs/fourn/index.php
+++ b/htdocs/fourn/index.php
@@ -31,11 +31,11 @@ require_once DOL_DOCUMENT_ROOT.'/categories/class/categorie.class.php';
 $langs->load("suppliers");
 $langs->load("orders");
 $langs->load("companies");
 
 // Security check
-$socid = isset($_GET["socid"])?$_GET["socid"]:'';
+$socid = GETPOST("socid", 'int');
 if ($user->societe_id) $socid=$user->societe_id;
 $result = restrictedArea($user, 'societe',$socid,'');
 
 
 /*
@@ -79,11 +79,11 @@ if ($resql)
 	$var=True;
 
 	while ($i < $num)
 	{
 		$row = $db->fetch_row($resql);
-		
+
 
 		print '<tr class="oddeven">';
 		print '<td>'.$langs->trans($commande->statuts[$row[1]]).'</td>';
 		print '<td align="center">'.$row[0].'</td>';
 		print '<td align="center"><a href="'.DOL_URL_ROOT.'/fourn/commande/list.php?statut='.$row[1].'">'.$commande->LibStatut($row[1],3).'</a></td>';
@@ -130,11 +130,11 @@ if (! empty($conf->fournisseur->enabled))
 
 			$i = 0;
 			$var = true;
 			while ($i < $num)
 			{
-				
+
 				$obj = $db->fetch_object($resql);
 				print '<tr class="oddeven"><td  class="nowrap">';
 				$commandestatic->id=$obj->rowid;
 				$commandestatic->ref=$obj->ref;
 				print $commandestatic->getNomUrl(1,'',16);
@@ -149,11 +149,11 @@ if (! empty($conf->fournisseur->enabled))
 				$i++;
 				$total += $obj->total_ttc;
 			}
 			if ($total>0)
 			{
-				
+
 				print '<tr class="liste_total"><td>'.$langs->trans("Total").'</td><td colspan="2" align="right">'.price($total)."</td></tr>";
 			}
 			print "</table>";
 			print "<br>\n";
 		}
@@ -188,11 +188,11 @@ if (! empty($conf->fournisseur->enabled) && $user->rights->fournisseur->facture-
 			$tot_ttc = 0;
 			$var = True;
 			while ($i < $num && $i < 20)
 			{
 				$obj = $db->fetch_object($resql);
-				
+
 				print '<tr class="oddeven"><td class="nowrap">';
 				$facturestatic->ref=$obj->ref;
 				$facturestatic->id=$obj->rowid;
 				$facturestatic->type=$obj->type;
 				print $facturestatic->getNomUrl(1,'');
@@ -261,11 +261,11 @@ if ($resql)
 
 	$var=True;
 
 	while ($obj = $db->fetch_object($resql) )
 	{
-		
+
 
 		print '<tr class="oddeven">';
 		print '<td><a href="card.php?socid='.$obj->socid.'">'.img_object($langs->trans("ShowSupplier"),"company").'</a>';
 		print "&nbsp;<a href=\"card.php?socid=".$obj->socid."\">".$obj->name."</a></td>\n";
 		print '<td align="left">'.$obj->code_fournisseur.'&nbsp;</td>';
@@ -298,11 +298,11 @@ if (count($companystatic->SupplierCategories))
 	print "</td></tr>\n";
 	$var=True;
 
 	foreach ($companystatic->SupplierCategories as $rowid => $label)
 	{
-		
+
 		print "<tr ".$bc[$var].">\n";
 		print '<td>';
 		$categstatic->id=$rowid;
 		$categstatic->ref=$label;
 		$categstatic->label=$label;
diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php
index 4febe121156..d5a692ca86e 100644
--- a/htdocs/main.inc.php
+++ b/htdocs/main.inc.php
@@ -79,11 +79,12 @@ function test_sql_and_script_inject($val, $type)
 {
     $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('/updatexml^(/i',	 $val);
+    	$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);

Vulnerable code: $rowid=isset($_GET["rowid"])?$_GET["rowid"]:$_POST["rowid"];
llxHeader();

$_socid = $_GET["id"];
	$soc->fetch($_GET["id"]);
	header("Location: multiprix.php?id=".$_GET["id"]);
	
	
	
$socid = isset($_GET["socid"])?$_GET["socid"]:'';
		
				
				
				
		
		
        $inj += preg_match('/delete\s+from/i',	 $val);
Fixed code: $rowid=GETPOST("rowid",'int');
llxHeader();

// Protection to avoid direct call of template
if (empty($langs) || ! is_object($langs))
{
	print "Error, template page can't be called as URL";
	exit;
}


// Protection to avoid direct call of template
if (empty($langs) || ! is_object($langs))
{
	print "Error, template page can't be called as URL";
	exit;
}


// Protection to avoid direct call of template
if (empty($langs) || ! is_object($langs))
{
	print "Error, template page can't be called as URL";
	exit;
}



// Protection to avoid direct call of template
if (empty($langs) || ! is_object($langs))
{
	print "Error, template page can't be called as URL";
	exit;
}


// Protection to avoid direct call of template
if (empty($langs) || ! is_object($langs))
{
	print "Error, template page can't be called as URL";
	exit;
}


// Protection to avoid direct call of template
if (empty($langs) || ! is_object($langs))
{
	print "Error, template page can't be called as URL";
	exit;
}


$id = GETPOST('id','int');
$_socid = GETPOST("id",'int');
	$soc->fetch($id);
	header("Location: multiprix.php?id=".$id);



$socid = GETPOST("socid", 'int');






    	$inj += preg_match('/updatexml^(/i',	 $val);
    	$inj += preg_match('/delete\s+from/i',	 $val);