CVE-2018-13447: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: SQL injection vulnerability in product/card.php in Dolibarr ERP/CRM version 7.0.3 allows remote attackers to execute arbitrary SQL commands via the statut 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: 2018-07-08T16:29Z
Repository: https://github.com/Dolibarr/dolibarr
Language: PHP
Files changed: htdocs/core/class/html.form.class.php, htdocs/main.inc.php, htdocs/product/card.php, htdocs/societe/ajax/company.php
Commit date: 2018-06-25 14:06:09 +0200
Commit message: FIX SQL Injections reported by mu shcor (ADLab of Venustech)
Diff: diff --git a/htdocs/core/class/html.form.class.php b/htdocs/core/class/html.form.class.php
index 57783c09885f..547bd8ad4e4a 100644
--- a/htdocs/core/class/html.form.class.php
+++ b/htdocs/core/class/html.form.class.php
@@ -1042,11 +1042,11 @@ class Form
 	 *  Output html form to select a third party.
 	 *  Note, you must use the select_company to get the component to select a third party. This function must only be called by select_company.
 	 *
 	 *	@param	string	$selected       Preselected type
 	 *	@param  string	$htmlname       Name of field in form
-	 *  @param  string	$filter         optional filters criteras (example: 's.rowid <> x', 's.client in (1,3)')
+	 *  @param  string	$filter         Optional filters criteras (example: 's.rowid <> x', 's.client in (1,3)')
 	 *	@param	string	$showempty		Add an empty field (Can be '1' or text to use on empty line like 'SelectThirdParty')
 	 * 	@param	int		$showtype		Show third party type in combolist (customer, prospect or supplier)
 	 * 	@param	int		$forcecombo		Force to use standard HTML select component without beautification
 	 *  @param	array	$events			Event options. Example: array(array('method'=>'getContacts', 'url'=>dol_buildpath('/core/ajax/contacts.php',1), 'htmlname'=>'contactid', 'params'=>array('add-customer-contact'=>'disabled')))
 	 *  @param	string	$filterkey		Filter on key value
@@ -1062,10 +1062,13 @@ class Form
 
 		$out='';
 		$num=0;
 		$outarray=array();
 
+		// Clean $filter that may contains sql conditions so sql code
+		if (function_exists('test_sql_and_script_inject')) $filter = test_sql_and_script_inject($filter, 3);
+
 		// On recherche les societes
 		$sql = "SELECT s.rowid, s.nom as name, s.name_alias, s.client, s.fournisseur, s.code_client, s.code_fournisseur";
 		$sql.= " FROM ".MAIN_DB_PREFIX ."societe as s";
 		if (!$user->rights->societe->client->voir && !$user->societe_id) $sql .= ", ".MAIN_DB_PREFIX."societe_commerciaux as sc";
 		$sql.= " WHERE s.entity IN (".getEntity('societe').")";
diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php
index e9b17ed23ba7..daa0dda21a56 100644
--- a/htdocs/main.inc.php
+++ b/htdocs/main.inc.php
@@ -70,28 +70,34 @@ if (function_exists('get_magic_quotes_gpc'))	// magic_quotes_* deprecated in PHP
 
 /**
  * Security: SQL Injection and XSS Injection (scripts) protection (Filters on GET, POST, PHP_SELF).
  *
  * @param		string		$val		Value
- * @param		string		$type		1=GET, 0=POST, 2=PHP_SELF
+ * @param		string		$type		1=GET, 0=POST, 2=PHP_SELF, 3=GET without sql reserved keywords (the less tolerant test)
  * @return		int						>0 if there is an injection, 0 if none
  */
 function test_sql_and_script_inject($val, $type)
 {
 	$inj = 0;
 	// For SQL Injection (only GET are used to be included into bad escaped SQL requests)
-	if ($type == 1)
+	if ($type == 1 || $type == 3)
 	{
-		$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('/insert\s+into/i', 	 $val);
 		$inj += preg_match('/select\s+from/i', 	 $val);
 		$inj += preg_match('/into\s+(outfile|dumpfile)/i',  $val);
+		$inj += preg_match('/user\s*\(/i',  $val);						// avoid to use function user() that return current database login
+		$inj += preg_match('/information_schema/i',  $val);				// avoid to use request that read information_schema database
 	}
-	if ($type != 2)	// Not common, we can check on POST
+	if ($type == 3)
 	{
+		$inj += preg_match('/select|update|delete|replace|group\s+by|concat|count|from/i',	 $val);
+	}
+	if ($type != 2)	// Not common key strings, so we can check them both on GET and POST
+	{
+		$inj += preg_match('/updatexml\(/i', 	 $val);
 		$inj += preg_match('/update.+set.+=/i',  $val);
 		$inj += preg_match('/union.+select/i', 	 $val);
 		$inj += preg_match('/(\.\.%2f)+/i',		 $val);
 	}
 	// For XSS Injection done by adding javascript with script
@@ -1556,12 +1562,10 @@ function top_menu($head, $title='', $target='', $disablejs=0, $disablehead=0, $a
 		print $toprightmenu;
 
 		print "</div>\n";
 		print '</div></div>';
 
-		//unset($form);
-
 		print '<div style="clear: both;"></div>';
 		print "<!-- End top horizontal menu -->\n\n";
 	}
 
 	if (empty($conf->dol_hide_leftmenu) && empty($conf->dol_use_jmobile)) print '<!-- Begin div id-container --><div id="id-container" class="id-container'.($morecss?' '.$morecss:'').'">';
diff --git a/htdocs/product/card.php b/htdocs/product/card.php
index e06d2c4569d2..1e6995fff359 100644
--- a/htdocs/product/card.php
+++ b/htdocs/product/card.php
@@ -287,12 +287,12 @@ if (empty($reshook))
 
             $object->description        	 = dol_htmlcleanlastbr(GETPOST('desc','none'));
             $object->url					 = GETPOST('url');
             $object->note_private          	 = dol_htmlcleanlastbr(GETPOST('note_private','none'));
             $object->note               	 = $object->note_private;   // deprecated
-            $object->customcode              = GETPOST('customcode');
-            $object->country_id              = GETPOST('country_id');
+            $object->customcode              = GETPOST('customcode','alpha');
+            $object->country_id              = GETPOST('country_id','int');
             $object->duration_value     	 = $duration_value;
             $object->duration_unit      	 = $duration_unit;
             $object->seuil_stock_alerte 	 = GETPOST('seuil_stock_alerte')?GETPOST('seuil_stock_alerte'):0;
             $object->desiredstock            = GETPOST('desiredstock')?GETPOST('desiredstock'):0;
             $object->canvas             	 = GETPOST('canvas');
@@ -304,17 +304,17 @@ if (empty($reshook))
             $object->height             	 = GETPOST('sizeheight');
 	        $object->surface            	 = GETPOST('surface');
             $object->surface_units      	 = GETPOST('surface_units');
             $object->volume             	 = GETPOST('volume');
             $object->volume_units       	 = GETPOST('volume_units');
-            $object->finished           	 = GETPOST('finished');
-	        $object->fk_unit                 = GETPOST('units');
+            $object->finished           	 = GETPOST('finished','alpha');
+            $object->fk_unit                 = GETPOST('units','alpha');
 
-			$accountancy_code_sell 			 = GETPOST('accountancy_code_sell');
-			$accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra');
-			$accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export');
-			$accountancy_code_buy 			 = GETPOST('accountancy_code_buy');
+	        $accountancy_code_sell 			 = GETPOST('accountancy_code_sell','alpha');
+	        $accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra','alpha');
+	        $accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export','alpha');
+	        $accountancy_code_buy 			 = GETPOST('accountancy_code_buy','alpha');
 
 			if ($accountancy_code_sell <= 0) { $object->accountancy_code_sell = ''; } else { $object->accountancy_code_sell = $accountancy_code_sell; }
 			if ($accountancy_code_sell_intra <= 0) { $object->accountancy_code_sell_intra = ''; } else { $object->accountancy_code_sell_intra = $accountancy_code_sell_intra; }
 			if ($accountancy_code_sell_export <= 0) { $object->accountancy_code_sell_export = ''; } else { $object->accountancy_code_sell_export = $accountancy_code_sell_export; }
 			if ($accountancy_code_buy <= 0) { $object->accountancy_code_buy = ''; } else { $object->accountancy_code_buy = $accountancy_code_buy; }
@@ -383,15 +383,15 @@ if (empty($reshook))
     			if (! empty($conf->global->MAIN_DISABLE_NOTES_TAB))
     			{
                 	$object->note_private           = dol_htmlcleanlastbr(GETPOST('note_private','none'));
                     $object->note                   = $object->note_private;
     			}
-                $object->customcode             = GETPOST('customcode');
-                $object->country_id             = GETPOST('country_id');
-                $object->status                 = GETPOST('statut');
-                $object->status_buy             = GETPOST('statut_buy');
-                $object->status_batch	        = GETPOST('status_batch');
+                $object->customcode             = GETPOST('customcode','alpha');
+                $object->country_id             = GETPOST('country_id','int');
+                $object->status                 = GETPOST('statut','int');
+                $object->status_buy             = GETPOST('statut_buy','int');
+                $object->status_batch	        = GETPOST('status_batch','aZ09');
                 // removed from update view so GETPOST always empty
                 /*
                 $object->seuil_stock_alerte     = GETPOST('seuil_stock_alerte');
                 $object->desiredstock           = GETPOST('desiredstock');
                 */
@@ -408,11 +408,11 @@ if (empty($reshook))
 
                 $object->surface                = GETPOST('surface');
                 $object->surface_units          = GETPOST('surface_units');
                 $object->volume                 = GETPOST('volume');
                 $object->volume_units           = GETPOST('volume_units');
-                $object->finished               = GETPOST('finished');
+                $object->finished               = GETPOST('finished','alpha');
 
 	            $units = GETPOST('units', 'int');
 
 	            if ($units > 0) {
 		            $object->fk_unit = $units;
@@ -435,14 +435,14 @@ if (empty($reshook))
     	        }
     	        $object->barcode_type_code      = $stdobject->barcode_type_code;
     	        $object->barcode_type_coder     = $stdobject->barcode_type_coder;
     	        $object->barcode_type_label     = $stdobject->barcode_type_label;
 
-				$accountancy_code_sell 			 = GETPOST('accountancy_code_sell');
-				$accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra');
-				$accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export');
-				$accountancy_code_buy 			 = GETPOST('accountancy_code_buy');
+    	        $accountancy_code_sell 			 = GETPOST('accountancy_code_sell','alpha');
+    	        $accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra','alpha');
+    	        $accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export','alpha');
+    	        $accountancy_code_buy 			 = GETPOST('accountancy_code_buy','alpha');
 
 				if ($accountancy_code_sell <= 0) { $object->accountancy_code_sell = ''; } else { $object->accountancy_code_sell = $accountancy_code_sell; }
 				if ($accountancy_code_sell_intra <= 0) { $object->accountancy_code_sell_intra = ''; } else { $object->accountancy_code_sell_intra = $accountancy_code_sell_intra; }
 				if ($accountancy_code_sell_export <= 0) { $object->accountancy_code_sell_export = ''; } else { $object->accountancy_code_sell_export = $accountancy_code_sell_export; }
 				if ($accountancy_code_buy <= 0) { $object->accountancy_code_buy = ''; } else { $object->accountancy_code_buy = $accountancy_code_buy; }
@@ -1301,11 +1301,11 @@ else
             }
             print '</select>';
             print '</td></tr>';
 
 			// Batch number managment
-			if ($conf->productbatch->enabled) 
+			if ($conf->productbatch->enabled)
 			{
 				if ($object->isProduct() || ! empty($conf->global->STOCK_SUPPORTS_SERVICES))
 				{
 					print '<tr><td>'.$langs->trans("ManageLotSerial").'</td><td colspan="3">';
 					$statutarray=array('0' => $langs->trans("ProductStatusNotOnBatch"), '1' => $langs->trans("ProductStatusOnBatch"));
@@ -1721,11 +1721,11 @@ else
 				print $object->accountancy_code_buy;
 			}
 			print '</td></tr>';
 
             // Batch number management (to batch)
-            if (! empty($conf->productbatch->enabled)) 
+            if (! empty($conf->productbatch->enabled))
             {
 				if ($object->isProduct() || ! empty($conf->global->STOCK_SUPPORTS_SERVICES))
 				{
             		print '<tr><td>'.$langs->trans("ManageLotSerial").'</td><td colspan="2">';
             	    if (! empty($conf->use_javascript_ajax) && $usercancreate && ! empty($conf->global->MAIN_DIRECT_STATUS_UPDATE)) {
diff --git a/htdocs/societe/ajax/company.php b/htdocs/societe/ajax/company.php
index 349e19b0c26a..d62c8a445d6c 100644
--- a/htdocs/societe/ajax/company.php
+++ b/htdocs/societe/ajax/company.php
@@ -85,11 +85,11 @@ else
 	// When used from jQuery, the search term is added as GET param "term".
 	$searchkey=(($id && GETPOST($id, 'alpha'))?GETPOST($id, 'alpha'):(($htmlname && GETPOST($htmlname, 'alpha'))?GETPOST($htmlname, 'alpha'):''));
 
 	if (! $searchkey) return;
 
-	$form = new Form($db);
+	if (! is_object($form)) $form = new Form($db);
 	$arrayresult=$form->select_thirdparty_list(0, $htmlname, $filter, 1, $showtype, 0, null, $searchkey, $outjson);
 
 	$db->close();
 
 	if ($outjson) print json_encode($arrayresult);

Vulnerable code: 	 *  @param  string	$filter         optional filters criteras (example: 's.rowid <> x', 's.client in (1,3)')
 * @param		string		$type		1=GET, 0=POST, 2=PHP_SELF
	if ($type == 1)
		$inj += preg_match('/updatexml\(/i',	 $val);
	if ($type != 2)	// Not common, we can check on POST
		//unset($form);

            $object->customcode              = GETPOST('customcode');
            $object->country_id              = GETPOST('country_id');
            $object->finished           	 = GETPOST('finished');
	        $object->fk_unit                 = GETPOST('units');
			$accountancy_code_sell 			 = GETPOST('accountancy_code_sell');
			$accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra');
			$accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export');
			$accountancy_code_buy 			 = GETPOST('accountancy_code_buy');
                $object->customcode             = GETPOST('customcode');
                $object->country_id             = GETPOST('country_id');
                $object->status                 = GETPOST('statut');
                $object->status_buy             = GETPOST('statut_buy');
                $object->status_batch	        = GETPOST('status_batch');
                $object->finished               = GETPOST('finished');
				$accountancy_code_sell 			 = GETPOST('accountancy_code_sell');
				$accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra');
				$accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export');
				$accountancy_code_buy 			 = GETPOST('accountancy_code_buy');
			if ($conf->productbatch->enabled) 
            if (! empty($conf->productbatch->enabled)) 
	$form = new Form($db);
Fixed code: 	 *  @param  string	$filter         Optional filters criteras (example: 's.rowid <> x', 's.client in (1,3)')
		// Clean $filter that may contains sql conditions so sql code
		if (function_exists('test_sql_and_script_inject')) $filter = test_sql_and_script_inject($filter, 3);

 * @param		string		$type		1=GET, 0=POST, 2=PHP_SELF, 3=GET without sql reserved keywords (the less tolerant test)
	if ($type == 1 || $type == 3)
		$inj += preg_match('/user\s*\(/i',  $val);						// avoid to use function user() that return current database login
		$inj += preg_match('/information_schema/i',  $val);				// avoid to use request that read information_schema database
	if ($type == 3)
		$inj += preg_match('/select|update|delete|replace|group\s+by|concat|count|from/i',	 $val);
	}
	if ($type != 2)	// Not common key strings, so we can check them both on GET and POST
	{
		$inj += preg_match('/updatexml\(/i', 	 $val);
            $object->customcode              = GETPOST('customcode','alpha');
            $object->country_id              = GETPOST('country_id','int');
            $object->finished           	 = GETPOST('finished','alpha');
            $object->fk_unit                 = GETPOST('units','alpha');
	        $accountancy_code_sell 			 = GETPOST('accountancy_code_sell','alpha');
	        $accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra','alpha');
	        $accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export','alpha');
	        $accountancy_code_buy 			 = GETPOST('accountancy_code_buy','alpha');
                $object->customcode             = GETPOST('customcode','alpha');
                $object->country_id             = GETPOST('country_id','int');
                $object->status                 = GETPOST('statut','int');
                $object->status_buy             = GETPOST('statut_buy','int');
                $object->status_batch	        = GETPOST('status_batch','aZ09');
                $object->finished               = GETPOST('finished','alpha');
    	        $accountancy_code_sell 			 = GETPOST('accountancy_code_sell','alpha');
    	        $accountancy_code_sell_intra	 = GETPOST('accountancy_code_sell_intra','alpha');
    	        $accountancy_code_sell_export	 = GETPOST('accountancy_code_sell_export','alpha');
    	        $accountancy_code_buy 			 = GETPOST('accountancy_code_buy','alpha');
			if ($conf->productbatch->enabled)
            if (! empty($conf->productbatch->enabled))
	if (! is_object($form)) $form = new Form($db);