From 36e6269b24b846efdf50c1736260709c16a31187 Mon Sep 17 00:00:00 2001 From: Regis Houssin Date: Fri, 26 Apr 2013 16:07:35 +0200 Subject: [PATCH] Fix: big security problem with multicompany --- htdocs/comm/action/class/actioncomm.class.php | 2 +- htdocs/comm/action/document.php | 2 ++ htdocs/comm/action/fiche.php | 2 +- htdocs/comm/action/info.php | 7 +++-- htdocs/core/lib/security.lib.php | 26 ++++++++++++++----- 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/htdocs/comm/action/class/actioncomm.class.php b/htdocs/comm/action/class/actioncomm.class.php index e39221921b0..dc78424568c 100644 --- a/htdocs/comm/action/class/actioncomm.class.php +++ b/htdocs/comm/action/class/actioncomm.class.php @@ -35,7 +35,7 @@ class ActionComm extends CommonObject public $element='action'; public $table_element = 'actioncomm'; public $table_rowid = 'id'; - protected $ismultientitymanaged = 2; // 0=No test on entity, 1=Test with field entity, 2=Test with link by societe + protected $ismultientitymanaged = 1; // 0=No test on entity, 1=Test with field entity, 2=Test with link by societe var $id; var $type_id; diff --git a/htdocs/comm/action/document.php b/htdocs/comm/action/document.php index 6a576c580ca..7ad74302655 100755 --- a/htdocs/comm/action/document.php +++ b/htdocs/comm/action/document.php @@ -51,6 +51,8 @@ if ($user->societe_id > 0) $socid = $user->societe_id; } +$result = restrictedArea($user, 'agenda', $objectid, 'actioncomm&societe', 'myactions&allactions', '', 'id'); + $act = new ActionComm($db); if ($objectid > 0) diff --git a/htdocs/comm/action/fiche.php b/htdocs/comm/action/fiche.php index aa2e9a695db..dd52d5d92a2 100644 --- a/htdocs/comm/action/fiche.php +++ b/htdocs/comm/action/fiche.php @@ -53,7 +53,7 @@ $contactid=GETPOST('contactid','int'); $socid = GETPOST('socid','int'); $id = GETPOST('id','int'); if ($user->societe_id) $socid=$user->societe_id; -//$result = restrictedArea($user, 'agenda', $id, 'actioncomm', 'actions', '', 'id'); +$result = restrictedArea($user, 'agenda', $id, 'actioncomm&societe', 'myactions&allactions', '', 'id'); $error=GETPOST("error"); $mesg=''; diff --git a/htdocs/comm/action/info.php b/htdocs/comm/action/info.php index 7c775bf78ac..66dc178d4b8 100644 --- a/htdocs/comm/action/info.php +++ b/htdocs/comm/action/info.php @@ -31,6 +31,8 @@ require_once DOL_DOCUMENT_ROOT.'/comm/action/class/actioncomm.class.php'; $langs->load("commercial"); +$id = GETPOST('id','int'); + // Security check if ($user->societe_id > 0) { @@ -38,6 +40,7 @@ if ($user->societe_id > 0) $socid = $user->societe_id; } +$result = restrictedArea($user, 'agenda', $id, 'actioncomm&societe', 'myactions&allactions', '', 'id'); /* @@ -48,8 +51,8 @@ $help_url='EN:Module_Agenda_En|FR:Module_Agenda|ES:M&omodulodulo_Agenda'; llxHeader('',$langs->trans("Agenda"),$help_url); $act = new ActionComm($db); -$act->fetch($_GET["id"]); -$act->info($_GET["id"]); +$act->fetch($id); +$act->info($act->id); $head=actions_prepare_head($act); dol_fiche_head($head, 'info', $langs->trans("Action"),0,'action'); diff --git a/htdocs/core/lib/security.lib.php b/htdocs/core/lib/security.lib.php index 896291fddef..0c7fdce0c99 100644 --- a/htdocs/core/lib/security.lib.php +++ b/htdocs/core/lib/security.lib.php @@ -112,11 +112,14 @@ function restrictedArea($user, $features, $objectid=0, $dbtablename='', $feature if (method_exists($objcanvas->control,'restrictedArea')) return $objcanvas->control->restrictedArea($user,$features,$objectid,$dbtablename,$feature2,$dbt_keyfield,$dbt_select); } - if ($dbt_select != 'rowid') $objectid = "'".$objectid."'"; + if ($dbt_select != 'rowid' && $dbt_select != 'id') $objectid = "'".$objectid."'"; // More features to check $features = explode("&", $features); + // More subfeatures to check + $feature2 = explode("&", $feature2); + // More parameters $params = explode('&', $dbtablename); $dbtablename=(! empty($params[0]) ? $params[0] : ''); @@ -164,8 +167,11 @@ function restrictedArea($user, $features, $objectid=0, $dbtablename='', $feature } 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; + foreach($feature2 as $subfeature) + { + if (empty($user->rights->$feature->$subfeature->lire) && empty($user->rights->$feature->$subfeature->read)) $readok=0; + else $readok=1; + } } else if (! empty($feature) && ($feature!='user' && $feature!='usergroup')) // This is for old permissions { @@ -210,8 +216,11 @@ function restrictedArea($user, $features, $objectid=0, $dbtablename='', $feature } 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; + foreach($feature2 as $subfeature) + { + if (empty($user->rights->$feature->$subfeature->creer) && empty($user->rights->$feature->$subfeature->write)) $createok=0; + else $createok=1; + } } else if (! empty($feature)) // This is for old permissions { @@ -271,8 +280,11 @@ function restrictedArea($user, $features, $objectid=0, $dbtablename='', $feature } else if (! empty($feature2)) // This should be used for future changes { - if (empty($user->rights->$feature->$feature2->supprimer) - && empty($user->rights->$feature->$feature2->delete)) $deleteok=0; + foreach($feature2 as $subfeature) + { + if (empty($user->rights->$feature->$subfeature->supprimer) && empty($user->rights->$feature->$subfeature->delete)) $deleteok=0; + else $deleteok=1; + } } else if (! empty($feature)) // This is for old permissions {