From 2d6444fb6801783782e37b60ac5f76cad9f8aaad Mon Sep 17 00:00:00 2001 From: Regis Houssin <regis@dolibarr.fr> Date: Fri, 16 Oct 2009 13:09:56 +0000 Subject: [PATCH] Security: More features to check --- htdocs/categories/categorie.php | 4 +- htdocs/lib/functions.lib.php | 282 ++++++++++++++++--------------- htdocs/lib/product.lib.php | 8 +- htdocs/product/barcode.php | 2 +- htdocs/product/fournisseurs.php | 2 +- htdocs/product/stock/product.php | 2 +- 6 files changed, 157 insertions(+), 143 deletions(-) diff --git a/htdocs/categories/categorie.php b/htdocs/categories/categorie.php index a0b0c3c3327..aabbbf9832d 100644 --- a/htdocs/categories/categorie.php +++ b/htdocs/categories/categorie.php @@ -39,14 +39,14 @@ if ($_REQUEST["socid"]) { if ($_REQUEST["typeid"] == 1) { $type = 'fournisseur'; $socid = isset($_REQUEST["socid"])?$_REQUEST["socid"]:''; } if ($_REQUEST["typeid"] == 2) { $type = 'societe'; $socid = isset($_REQUEST["socid"])?$_REQUEST["socid"]:''; } - $objecttype = 'societe'; + $objecttype = 'societe&&categorie'; $objectid = isset($_REQUEST["socid"])?$_REQUEST["socid"]:''; $fieldid = 'rowid'; } else if ($_REQUEST["id"] || $_REQUEST["ref"]) { $type = 'produit'; - $objecttype = 'produit|service'; + $objecttype = 'produit|service&&categorie'; $objectid = isset($_REQUEST["id"])?$_REQUEST["id"]:(isset($_REQUEST["ref"])?$_REQUEST["ref"]:''); $dbtablename = 'product'; $fieldid = isset($_REQUEST["ref"])?'ref':'rowid'; diff --git a/htdocs/lib/functions.lib.php b/htdocs/lib/functions.lib.php index 7e495054b0d..a6094a3f220 100644 --- a/htdocs/lib/functions.lib.php +++ b/htdocs/lib/functions.lib.php @@ -1499,206 +1499,220 @@ function info_admin($texte,$infoonimgalt=0) /** * \brief Check permissions of a user to show a page and an object. * \param user User to check - * \param feature Feature to check (in most cases, it's module name) + * \param features Features to check (in most cases, it's module name) * \param objectid Object ID if we want to check permission on on object (optionnal) * \param dbtablename Table name where object is stored. Not used if objectid is null (optionnal) * \param feature2 Feature to check (second level of permission) * \param dbt_keyfield Field name for socid foreign key if not fk_soc. (optionnal) * \param dbt_select Field name for select if not rowid. (optionnal) */ -function restrictedArea($user, $feature='societe', $objectid=0, $dbtablename='', $feature2='', $dbt_keyfield='fk_soc', $dbt_select='rowid') +function restrictedArea($user, $features='societe', $objectid=0, $dbtablename='', $feature2='', $dbt_keyfield='fk_soc', $dbt_select='rowid') { global $db, $conf; //dol_syslog("functions.lib:restrictedArea $feature, $objectid, $dbtablename,$feature2,$dbt_socfield,$dbt_select"); if ($dbt_select != 'rowid') $objectid = "'".$objectid."'"; - //print "user_id=".$user->id.", feature=".$feature.", feature2=".$feature2.", object_id=".$objectid; + //print "user_id=".$user->id.", features=".$features.", feature2=".$feature2.", object_id=".$objectid; //print ", dbtablename=".$dbtablename.", dbt_socfield=".$dbt_keyfield.", dbt_select=".$dbt_select; //print ", user_societe_contact_lire=".$user->rights->societe->contact->lire."<br>"; - + + // More features to check + $features = explode("&&",$features); + // Check read permission from module // TODO Replace "feature" param by permission for reading $readok=1; - if ($feature == 'societe') - { - if (! $user->rights->societe->lire && ! $user->rights->fournisseur->lire) $readok=0; - } - else if ($feature == 'contact') - { - if (! $user->rights->societe->contact->lire) $readok=0; - } - else if ($feature == 'produit|service') - { - if (! $user->rights->produit->lire && ! $user->rights->service->lire) $readok=0; - } - else if ($feature == 'prelevement') - { - if (! $user->rights->prelevement->bons->lire) $readok=0; - } - else if ($feature == 'commande_fournisseur') - { - if (! $user->rights->fournisseur->commande->lire) $readok=0; - } - else if ($feature == 'cheque') - { - if (! $user->rights->banque->cheque) $readok=0; - } - else if ($feature == 'ecm') - { - if (! $user->rights->ecm->download) $readok=0; - } - else if (! empty($feature2)) // This should be used for future changes - { - if (empty($user->rights->$feature->$feature2->lire) - && empty($user->rights->$feature->$feature2->read)) $readok=0; - } - else if (! empty($feature) && ($feature!='user' && $feature!='usergroup')) // This is for old permissions - { - if (empty($user->rights->$feature->lire) - && empty($user->rights->$feature->read)) $readok=0; - } - if (! $readok) - { - //print "Read access is down"; - accessforbidden(); - } - //print "Read access is ok"; - - // Check write permission from module - $createok=1; - if ( (isset($_GET["action"]) && $_GET["action"] == 'create') - || (isset($_POST["action"]) && $_POST["action"] == 'create') ) + foreach ($features as $feature) { if ($feature == 'societe') { - if (! $user->rights->societe->creer && ! $user->rights->fournisseur->creer) $createok=0; + if (! $user->rights->societe->lire && ! $user->rights->fournisseur->lire) $readok=0; } else if ($feature == 'contact') { - if (! $user->rights->societe->contact->creer) $createok=0; + if (! $user->rights->societe->contact->lire) $readok=0; } else if ($feature == 'produit|service') { - if (! $user->rights->produit->creer && ! $user->rights->service->creer) $createok=0; + if (! $user->rights->produit->lire && ! $user->rights->service->lire) $readok=0; } else if ($feature == 'prelevement') { - if (! $user->rights->prelevement->bons->creer) $createok=0; + if (! $user->rights->prelevement->bons->lire) $readok=0; } else if ($feature == 'commande_fournisseur') { - if (! $user->rights->fournisseur->commande->creer) $createok=0; + if (! $user->rights->fournisseur->commande->lire) $readok=0; } - else if ($feature == 'banque') + else if ($feature == 'cheque') { - if (! $user->rights->banque->modifier) $createok=0; + if (! $user->rights->banque->cheque) $readok=0; } - else if ($feature == 'cheque') + else if ($feature == 'ecm') { - if (! $user->rights->banque->cheque) $createok=0; + if (! $user->rights->ecm->download) $readok=0; } else if (! empty($feature2)) // This should be used for future changes { - if (empty($user->rights->$feature->$feature2->creer) - && empty($user->rights->$feature->$feature2->write)) $createok=0; + if (empty($user->rights->$feature->$feature2->lire) + && empty($user->rights->$feature->$feature2->read)) $readok=0; } - else if (! empty($feature)) // This is for old permissions + else if (! empty($feature) && ($feature!='user' && $feature!='usergroup')) // This is for old permissions { - if (empty($user->rights->$feature->creer) - && empty($user->rights->$feature->write)) $createok=0; + if (empty($user->rights->$feature->lire) + && empty($user->rights->$feature->read)) $readok=0; } - if (! $createok) accessforbidden(); - //print "Write access is ok"; } - // If we have a particular object to check permissions on - if ($objectid > 0) + if (! $readok) { - $sql=''; - - // If dbtable not defined, we use same name for table than module name - if (empty($dbtablename)) $dbtablename = $feature; + //print "Read access is down"; + accessforbidden(); + } + //print "Read access is ok"; - // Check permission for object with entity - if ($feature == 'user' || $feature == 'usergroup' || $feature == 'produit' || $feature == 'service' || $feature == 'produit|service') - { - $sql = "SELECT dbt.".$dbt_select; - $sql.= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt"; - $sql.= " WHERE dbt.".$dbt_select." = ".$objectid; - $sql.= " AND dbt.entity IN (0,".$conf->entity.")"; - } - else if ($feature == 'societe') + // Check write permission from module + $createok=1; + if ( (isset($_GET["action"]) && $_GET["action"] == 'create') + || (isset($_POST["action"]) && $_POST["action"] == 'create') ) + { + foreach ($features as $feature) { - // If external user: Check permission for external users - if ($user->societe_id > 0) + if ($feature == 'societe') { - if ($user->societe_id <> $objectid) accessforbidden(); + if (! $user->rights->societe->creer && ! $user->rights->fournisseur->creer) $createok=0; } - // If internal user: Check permission for internal users that are restricted on their objects - else if (! $user->rights->societe->client->voir) + else if ($feature == 'contact') { - $sql = "SELECT sc.fk_soc"; - $sql.= " FROM (".MAIN_DB_PREFIX."societe_commerciaux as sc"; - $sql.= ", ".MAIN_DB_PREFIX."societe as s)"; - $sql.= " WHERE sc.fk_soc = ".$objectid; - $sql.= " AND sc.fk_user = ".$user->id; - $sql.= " AND sc.fk_soc = s.rowid"; - $sql.= " AND s.entity = ".$conf->entity; + if (! $user->rights->societe->contact->creer) $createok=0; } - // If multicompany and internal users with all permissions, check user is in correct entity - else if ($conf->global->MAIN_MODULE_MULTICOMPANY) + else if ($feature == 'produit|service') { - $sql = "SELECT s.rowid"; - $sql.= " FROM ".MAIN_DB_PREFIX."societe as s"; - $sql.= " WHERE s.rowid = ".$objectid; - $sql.= " AND s.entity = ".$conf->entity; + if (! $user->rights->produit->creer && ! $user->rights->service->creer) $createok=0; } - } - else - { - // If external user: Check permission for external users - if ($user->societe_id > 0) + else if ($feature == 'prelevement') { - $sql = "SELECT dbt.fk_soc"; - $sql.= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt"; - $sql.= " WHERE dbt.rowid = ".$objectid; - $sql.= " AND dbt.fk_soc = ".$user->societe_id; + if (! $user->rights->prelevement->bons->creer) $createok=0; } - // If internal user: Check permission for internal users that are restricted on their objects - else if (! $user->rights->societe->client->voir) + else if ($feature == 'commande_fournisseur') { - $sql = "SELECT sc.fk_soc"; - $sql.= " FROM (".MAIN_DB_PREFIX.$dbtablename." as dbt"; - $sql.= ", ".MAIN_DB_PREFIX."societe as s)"; - $sql.= " LEFT JOIN ".MAIN_DB_PREFIX."societe_commerciaux as sc ON sc.fk_soc = dbt.".$dbt_keyfield; - $sql.= " WHERE dbt.rowid = ".$objectid; - $sql.= " AND dbt.fk_soc = s.rowid"; - $sql.= " AND s.entity = ".$conf->entity; - $sql.= " AND IFNULL(sc.fk_user, ".$user->id.") = ".$user->id; + if (! $user->rights->fournisseur->commande->creer) $createok=0; } - // If multicompany and internal users with all permissions, check user is in correct entity - else if ($conf->global->MAIN_MODULE_MULTICOMPANY) + else if ($feature == 'banque') { - $sql = "SELECT dbt.".$dbt_select; - $sql.= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt"; - $sql.= " WHERE dbt.".$dbt_select." = ".$objectid; - $sql.= " AND dbt.entity = ".$conf->entity; + if (! $user->rights->banque->modifier) $createok=0; + } + else if ($feature == 'cheque') + { + if (! $user->rights->banque->cheque) $createok=0; + } + else if (! empty($feature2)) // This should be used for future changes + { + if (empty($user->rights->$feature->$feature2->creer) + && empty($user->rights->$feature->$feature2->write)) $createok=0; + } + else if (! empty($feature)) // This is for old permissions + { + if (empty($user->rights->$feature->creer) + && empty($user->rights->$feature->write)) $createok=0; } } + + if (! $createok) accessforbidden(); + //print "Write access is ok"; + } - //print $sql."<br>"; - if ($sql) + // If we have a particular object to check permissions on + if ($objectid > 0) + { + foreach ($features as $feature) { - $resql=$db->query($sql); - if ($resql) + $sql=''; + + // If dbtable not defined, we use same name for table than module name + if (empty($dbtablename)) $dbtablename = $feature; + + // Check permission for object with entity + if ($feature == 'user' || $feature == 'usergroup' || $feature == 'produit' || $feature == 'service' || $feature == 'produit|service') { - if ($db->num_rows($resql) == 0) accessforbidden(); + $sql = "SELECT dbt.".$dbt_select; + $sql.= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt"; + $sql.= " WHERE dbt.".$dbt_select." = ".$objectid; + $sql.= " AND dbt.entity IN (0,".$conf->entity.")"; + } + else if ($feature == 'societe') + { + // If external user: Check permission for external users + if ($user->societe_id > 0) + { + if ($user->societe_id <> $objectid) accessforbidden(); + } + // If internal user: Check permission for internal users that are restricted on their objects + else if (! $user->rights->societe->client->voir) + { + $sql = "SELECT sc.fk_soc"; + $sql.= " FROM (".MAIN_DB_PREFIX."societe_commerciaux as sc"; + $sql.= ", ".MAIN_DB_PREFIX."societe as s)"; + $sql.= " WHERE sc.fk_soc = ".$objectid; + $sql.= " AND sc.fk_user = ".$user->id; + $sql.= " AND sc.fk_soc = s.rowid"; + $sql.= " AND s.entity = ".$conf->entity; + } + // If multicompany and internal users with all permissions, check user is in correct entity + else if ($conf->global->MAIN_MODULE_MULTICOMPANY) + { + $sql = "SELECT s.rowid"; + $sql.= " FROM ".MAIN_DB_PREFIX."societe as s"; + $sql.= " WHERE s.rowid = ".$objectid; + $sql.= " AND s.entity = ".$conf->entity; + } } else { - dol_syslog("functions.lib:restrictedArea sql=".$sql, LOG_ERR); - accessforbidden(); + // If external user: Check permission for external users + if ($user->societe_id > 0) + { + $sql = "SELECT dbt.fk_soc"; + $sql.= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt"; + $sql.= " WHERE dbt.rowid = ".$objectid; + $sql.= " AND dbt.fk_soc = ".$user->societe_id; + } + // If internal user: Check permission for internal users that are restricted on their objects + else if (! $user->rights->societe->client->voir) + { + $sql = "SELECT sc.fk_soc"; + $sql.= " FROM (".MAIN_DB_PREFIX.$dbtablename." as dbt"; + $sql.= ", ".MAIN_DB_PREFIX."societe as s)"; + $sql.= " LEFT JOIN ".MAIN_DB_PREFIX."societe_commerciaux as sc ON sc.fk_soc = dbt.".$dbt_keyfield; + $sql.= " WHERE dbt.rowid = ".$objectid; + $sql.= " AND dbt.fk_soc = s.rowid"; + $sql.= " AND s.entity = ".$conf->entity; + $sql.= " AND IFNULL(sc.fk_user, ".$user->id.") = ".$user->id; + } + // If multicompany and internal users with all permissions, check user is in correct entity + else if ($conf->global->MAIN_MODULE_MULTICOMPANY) + { + $sql = "SELECT dbt.".$dbt_select; + $sql.= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt"; + $sql.= " WHERE dbt.".$dbt_select." = ".$objectid; + $sql.= " AND dbt.entity = ".$conf->entity; + } + } + + //print $sql."<br>"; + if ($sql) + { + $resql=$db->query($sql); + if ($resql) + { + if ($db->num_rows($resql) == 0) accessforbidden(); + } + else + { + dol_syslog("functions.lib:restrictedArea sql=".$sql, LOG_ERR); + accessforbidden(); + } } } } diff --git a/htdocs/lib/product.lib.php b/htdocs/lib/product.lib.php index 12a547c42ce..830e0db87b2 100644 --- a/htdocs/lib/product.lib.php +++ b/htdocs/lib/product.lib.php @@ -50,7 +50,7 @@ function product_prepare_head($product, $user) $h++; // Show category tab - if ($conf->categorie->enabled) + if ($conf->categorie->enabled && $user->rights->categorie->lire) { $head[$h][0] = DOL_URL_ROOT."/categories/categorie.php?id=".$product->id; $head[$h][1] = $langs->trans('Categories'); @@ -59,7 +59,7 @@ function product_prepare_head($product, $user) } // Show barcode tab - if ($conf->global->MAIN_MODULE_BARCODE) + if ($conf->global->MAIN_MODULE_BARCODE && $user->rights->barcode->lire) { $head[$h][0] = DOL_URL_ROOT."/product/barcode.php?id=".$product->id; $head[$h][1] = $langs->trans("BarCode"); @@ -85,7 +85,7 @@ function product_prepare_head($product, $user) $h++; } - if ($conf->fournisseur->enabled) + if ($conf->fournisseur->enabled && $user->rights->fournisseur->lire) { $head[$h][0] = DOL_URL_ROOT."/product/fournisseurs.php?id=".$product->id; $head[$h][1] = $langs->trans("Suppliers"); @@ -110,7 +110,7 @@ function product_prepare_head($product, $user) if($product->isproduct()) // Si produit stockable { - if ($conf->stock->enabled) + if ($conf->stock->enabled && $user->rights->stock->lire) { $head[$h][0] = DOL_URL_ROOT."/product/stock/product.php?id=".$product->id; $head[$h][1] = $langs->trans("Stock"); diff --git a/htdocs/product/barcode.php b/htdocs/product/barcode.php index ccc046a31fe..cb765b50b45 100644 --- a/htdocs/product/barcode.php +++ b/htdocs/product/barcode.php @@ -40,7 +40,7 @@ if (isset($_GET["id"]) || isset($_GET["ref"])) } $fieldid = isset($_GET["ref"])?'ref':'rowid'; if ($user->societe_id) $socid=$user->societe_id; -$result=restrictedArea($user,'produit|service',$id,'product','','',$fieldid); +$result=restrictedArea($user,'produit|service&&barcode',$id,'product','','',$fieldid); /* * Actions diff --git a/htdocs/product/fournisseurs.php b/htdocs/product/fournisseurs.php index eaea6d1cd07..01598cb1b19 100644 --- a/htdocs/product/fournisseurs.php +++ b/htdocs/product/fournisseurs.php @@ -50,7 +50,7 @@ if (isset($_GET["id"]) || isset($_GET["ref"])) } $fieldid = isset($_GET["ref"])?'ref':'rowid'; if ($user->societe_id) $socid=$user->societe_id; -$result=restrictedArea($user,'produit|service',$id,'product','','',$fieldid); +$result=restrictedArea($user,'produit|service&&fournisseur',$id,'product','','',$fieldid); $mesg = ''; diff --git a/htdocs/product/stock/product.php b/htdocs/product/stock/product.php index ea8f91016c0..a01e1538db7 100644 --- a/htdocs/product/stock/product.php +++ b/htdocs/product/stock/product.php @@ -43,7 +43,7 @@ if (isset($_GET["id"]) || isset($_GET["ref"])) } $fieldid = isset($_GET["ref"])?'ref':'rowid'; if ($user->societe_id) $socid=$user->societe_id; -$result=restrictedArea($user,'produit',$id,'product','','',$fieldid); +$result=restrictedArea($user,'produit&&stock',$id,'product','','',$fieldid); $mesg = ''; -- GitLab