Skip to content

Commit

Permalink
FIX SQL Injections reported by mu shcor (ADLab of Venustech)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eldy
eldy committed Jun 25, 2018
1 parent d9fc1e0 commit 36402c2
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 28 deletions.
5 changes: 4 additions & 1 deletion htdocs/core/class/html.form.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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";
Expand Down
16 changes: 10 additions & 6 deletions htdocs/main.inc.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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";
}
Expand Down
40 changes: 20 additions & 20 deletions htdocs/product/card.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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; }
Expand Down Expand Up @@ -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');
Expand All @@ -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');

Expand All @@ -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; }
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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))
{
Expand Down
2 changes: 1 addition & 1 deletion htdocs/societe/ajax/company.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

1 comment on commit 36402c2

@fgeek
Copy link

@fgeek fgeek commented on 36402c2 Jul 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you release new version including these fixes?

Please sign in to comment.