Security advisory GHSA-24q2-6x37-cgcx: Dolibarr SQL injection vulnerability in product/card.php
Package: dolibarr/dolibarr (composer), vulnerable version: 7.0.3, fixed in: 7.0.4
CWE: CWE-89
Files changed by the fix:
htdocs/core/class/html.form.class.php
htdocs/main.inc.php
htdocs/product/card.php
htdocs/societe/ajax/company.php

Fix diff:
diff --git a/htdocs/core/class/html.form.class.php b/htdocs/core/class/html.form.class.php
index 57783c09885f1..547bd8ad4e4a4 100644
--- a/htdocs/core/class/html.form.class.php
+++ b/htdocs/core/class/html.form.class.php
@@ -1044,7 +1044,7 @@ function select_company($selected='', $htmlname='socid', $filter='', $showempty=
 	 *
 	 *	@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
@@ -1064,6 +1064,9 @@ function select_thirdparty_list($selected='',$htmlname='socid',$filter='',$showe
 		$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";
diff --git a/htdocs/main.inc.php b/htdocs/main.inc.php
index e9b17ed23ba75..daa0dda21a565 100644
--- a/htdocs/main.inc.php
+++ b/htdocs/main.inc.php
@@ -72,24 +72,30 @@ function stripslashes_deep($value)
  * 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);
@@ -1558,8 +1564,6 @@ function top_menu($head, $title='', $target='', $disablejs=0, $disablehead=0, $a
 		print "</div>\n";
 		print '</div></div>';
 
-		//unset($form);
-
 		print '<div style="clear: both;"></div>';
 		print "<!-- End top horizontal menu -->\n\n";
 	}
diff --git a/htdocs/product/card.php b/htdocs/product/card.php
index e06d2c4569d24..1e6995fff359a 100644
--- a/htdocs/product/card.php
+++ b/htdocs/product/card.php
@@ -289,8 +289,8 @@
             $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;
@@ -306,13 +306,13 @@
             $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; }
@@ -385,11 +385,11 @@
                 	$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');
@@ -410,7 +410,7 @@
                 $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');
 
@@ -437,10 +437,10 @@
     	        $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; }
@@ -1303,7 +1303,7 @@
             print '</td></tr>';
 
 			// Batch number managment
-			if ($conf->productbatch->enabled) 
+			if ($conf->productbatch->enabled)
 			{
 				if ($object->isProduct() || ! empty($conf->global->STOCK_SUPPORTS_SERVICES))
 				{
@@ -1723,7 +1723,7 @@
 			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))
 				{
diff --git a/htdocs/societe/ajax/company.php b/htdocs/societe/ajax/company.php
index 349e19b0c26ae..d62c8a445d6ce 100644
--- a/htdocs/societe/ajax/company.php
+++ b/htdocs/societe/ajax/company.php
@@ -87,7 +87,7 @@
 
 	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();