From b6dbe45242a1b1e9303b6103bf68e53535b85ad5 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Tue, 18 May 2021 01:58:54 +0200 Subject: [PATCH] Fix permissions on page to move position of file --- htdocs/adherents/document.php | 2 ++ htdocs/asset/document.php | 14 ++++++----- htdocs/bom/bom_document.php | 2 ++ htdocs/comm/action/document.php | 2 ++ htdocs/comm/propal/document.php | 2 ++ htdocs/commande/document.php | 14 ++++++----- htdocs/compta/facture/document.php | 2 ++ htdocs/core/actions_linkedfiles.inc.php | 11 +++++---- htdocs/core/ajax/row.php | 24 +++++++++++++------ htdocs/core/lib/security.lib.php | 2 +- .../template/myobject_document.php | 2 +- htdocs/product/document.php | 4 ++-- htdocs/projet/document.php | 10 ++++---- htdocs/societe/document.php | 1 + htdocs/ticket/agenda.php | 5 +--- htdocs/ticket/card.php | 4 ++-- htdocs/ticket/contact.php | 23 ++++++++++++++---- htdocs/ticket/document.php | 20 ++++++++++++---- htdocs/ticket/messaging.php | 9 +++---- 19 files changed, 98 insertions(+), 55 deletions(-) diff --git a/htdocs/adherents/document.php b/htdocs/adherents/document.php index 5150bea861e..f2a7c6f3f55 100644 --- a/htdocs/adherents/document.php +++ b/htdocs/adherents/document.php @@ -93,6 +93,8 @@ if ($id) { $caneditfieldmember = $user->rights->adherent->creer; } +$permissiontoadd = $canaddmember; + // Security check $result = restrictedArea($user, 'adherent', $object->id, '', '', 'socid', 'rowid', 0); diff --git a/htdocs/asset/document.php b/htdocs/asset/document.php index e6f5ed9b353..91e46269994 100644 --- a/htdocs/asset/document.php +++ b/htdocs/asset/document.php @@ -40,12 +40,6 @@ $socid = GETPOST('socid', 'int'); $action = GETPOST('action', 'aZ09'); $confirm = GETPOST('confirm', 'alpha'); -// Security check -if ($user->socid) { - $socid = $user->socid; -} -$result=restrictedArea($user, 'asset', $id, ''); - // Get parameters $limit = GETPOST('limit', 'int') ? GETPOST('limit', 'int') : $conf->liste_limit; $sortfield = GETPOST("sortfield", 'alpha'); @@ -69,6 +63,14 @@ if ($object->fetch($id)) { $upload_dir = $conf->asset->dir_output."/".dol_sanitizeFileName($object->ref); } +$permissiontoadd = $user->rights->asset->write; // Used by the include of actions_addupdatedelete.inc.php and actions_linkedfiles.inc.php + +// Security check +if ($user->socid) { + $socid = $user->socid; +} +$result=restrictedArea($user, 'asset', $id, ''); + /* * Actions diff --git a/htdocs/bom/bom_document.php b/htdocs/bom/bom_document.php index c0196670cfb..e59b22c45b2 100644 --- a/htdocs/bom/bom_document.php +++ b/htdocs/bom/bom_document.php @@ -85,6 +85,8 @@ if ($id > 0 || !empty($ref)) { $isdraft = (($object->status == $object::STATUS_DRAFT) ? 1 : 0); restrictedArea($user, 'bom', $object->id, 'bom_bom', '', '', 'rowid', $isdraft); +$permissiontoadd = $user->rights->bom->write; // Used by the include of actions_addupdatedelete.inc.php and actions_linkedfiles.inc.php + /* * Actions diff --git a/htdocs/comm/action/document.php b/htdocs/comm/action/document.php index f521fa2ab59..c55d4e114cc 100644 --- a/htdocs/comm/action/document.php +++ b/htdocs/comm/action/document.php @@ -88,6 +88,8 @@ if ($user->socid && $socid) { $result = restrictedArea($user, 'societe', $socid); } +$permissiontoadd = $user->rights->agenda->myactions->read; // Used by the include of actions_addupdatedelete.inc.php and actions_linkedfiles.inc.php + /* * Actions diff --git a/htdocs/comm/propal/document.php b/htdocs/comm/propal/document.php index 4e592c8371e..2b21c545c63 100644 --- a/htdocs/comm/propal/document.php +++ b/htdocs/comm/propal/document.php @@ -80,6 +80,8 @@ if (!$sortfield) { $object = new Propal($db); $object->fetch($id, $ref); +$permissiontoadd = $user->rights->propale->creer; + // Security check if (!empty($user->socid)) { $socid = $user->socid; diff --git a/htdocs/commande/document.php b/htdocs/commande/document.php index b4dddc9a1bf..c289112ee9f 100644 --- a/htdocs/commande/document.php +++ b/htdocs/commande/document.php @@ -44,12 +44,6 @@ $confirm = GETPOST('confirm'); $id = GETPOST('id', 'int'); $ref = GETPOST('ref'); -// Security check -if ($user->socid) { - $socid = $user->socid; -} -$result = restrictedArea($user, 'commande', $id, ''); - // Get parameters $limit = GETPOST('limit', 'int') ? GETPOST('limit', 'int') : $conf->liste_limit; $sortfield = GETPOST("sortfield", 'alpha'); @@ -78,6 +72,14 @@ if (!$sortfield) { $object = new Commande($db); +$permissiontoadd = $user->rights->commande->creer; + +// Security check +if ($user->socid) { + $socid = $user->socid; +} +$result = restrictedArea($user, 'commande', $id, ''); + /* * Actions diff --git a/htdocs/compta/facture/document.php b/htdocs/compta/facture/document.php index 664b84d4444..16cdee4ac53 100644 --- a/htdocs/compta/facture/document.php +++ b/htdocs/compta/facture/document.php @@ -72,6 +72,8 @@ if ($object->fetch($id, $ref)) { $upload_dir = $conf->facture->dir_output."/".dol_sanitizeFileName($object->ref); } +$permissiontoadd = $user->rights->facture->creer; + // Security check if ($user->socid) { $socid = $user->socid; diff --git a/htdocs/core/actions_linkedfiles.inc.php b/htdocs/core/actions_linkedfiles.inc.php index 13814511297..750ed2b2d9a 100644 --- a/htdocs/core/actions_linkedfiles.inc.php +++ b/htdocs/core/actions_linkedfiles.inc.php @@ -21,13 +21,14 @@ // Variable $upload_dir must be defined when entering here. // Variable $upload_dirold may also exists. // Variable $confirm must be defined. +// If variable $permissiontoadd is defined, we check it is true. Note: A test on permission should already have been done into the restrictedArea() method called by parent page. //var_dump($upload_dir); //var_dump($upload_dirold); // Submit file/link -if (GETPOST('sendit', 'alpha') && !empty($conf->global->MAIN_UPLOAD_DOC)) { +if (GETPOST('sendit', 'alpha') && !empty($conf->global->MAIN_UPLOAD_DOC) && (!isset($permissiontoadd) || $permissiontoadd)) { if (!empty($_FILES)) { if (is_array($_FILES['userfile']['tmp_name'])) { $userfiles = $_FILES['userfile']['tmp_name']; @@ -65,7 +66,7 @@ if (GETPOST('sendit', 'alpha') && !empty($conf->global->MAIN_UPLOAD_DOC)) { } } } -} elseif (GETPOST('linkit', 'restricthtml') && !empty($conf->global->MAIN_UPLOAD_DOC)) { +} elseif (GETPOST('linkit', 'restricthtml') && !empty($conf->global->MAIN_UPLOAD_DOC) && (!isset($permissiontoadd) || $permissiontoadd)) { $link = GETPOST('link', 'alpha'); if ($link) { if (substr($link, 0, 7) != 'http://' && substr($link, 0, 8) != 'https://' && substr($link, 0, 7) != 'file://' && substr($link, 0, 7) != 'davs://') { @@ -77,7 +78,7 @@ if (GETPOST('sendit', 'alpha') && !empty($conf->global->MAIN_UPLOAD_DOC)) { // Delete file/link -if ($action == 'confirm_deletefile' && $confirm == 'yes') { +if ($action == 'confirm_deletefile' && $confirm == 'yes' && (!isset($permissiontoadd) || $permissiontoadd)) { $urlfile = GETPOST('urlfile', 'alpha', 0, null, null, 1); // Do not use urldecode here ($_GET and $_REQUEST are already decoded by PHP). if (GETPOST('section', 'alpha')) { // For a delete from the ECM module, upload_dir is ECM root dir and urlfile contains relative path from upload_dir @@ -149,7 +150,7 @@ if ($action == 'confirm_deletefile' && $confirm == 'yes') { exit; } } -} elseif ($action == 'confirm_updateline' && GETPOST('save', 'alpha') && GETPOST('link', 'alpha')) { +} elseif ($action == 'confirm_updateline' && GETPOST('save', 'alpha') && GETPOST('link', 'alpha') && (!isset($permissiontoadd) || $permissiontoadd)) { require_once DOL_DOCUMENT_ROOT.'/core/class/link.class.php'; $langs->load('link'); $link = new Link($db); @@ -167,7 +168,7 @@ if ($action == 'confirm_deletefile' && $confirm == 'yes') { } else { //error fetching } -} elseif ($action == 'renamefile' && GETPOST('renamefilesave', 'alpha')) { +} elseif ($action == 'renamefile' && GETPOST('renamefilesave', 'alpha') && (!isset($permissiontoadd) || $permissiontoadd)) { // For documents pages, upload_dir contains already path to file from module dir, so we clean path into urlfile. if (!empty($upload_dir)) { $filenamefrom = dol_sanitizeFileName(GETPOST('renamefilefrom', 'alpha'), '_', 0); // Do not remove accents diff --git a/htdocs/core/ajax/row.php b/htdocs/core/ajax/row.php index 4662c3a1406..96dc792938f 100644 --- a/htdocs/core/ajax/row.php +++ b/htdocs/core/ajax/row.php @@ -49,6 +49,9 @@ if (!defined('NOREQUIRETRAN')) { require '../../main.inc.php'; require_once DOL_DOCUMENT_ROOT.'/core/class/genericobject.class.php'; +// Security check +// This is done later into view. + /* * View @@ -59,16 +62,16 @@ top_httphead(); print ''."\n"; // Registering the location of boxes -if (GETPOST('roworder', 'alpha', 2) && GETPOST('table_element_line', 'aZ09', 2) - && GETPOST('fk_element', 'aZ09', 2) && GETPOST('element_id', 'int', 2)) { - $roworder = GETPOST('roworder', 'alpha', 2); - $table_element_line = GETPOST('table_element_line', 'aZ09', 2); - $fk_element = GETPOST('fk_element', 'aZ09', 2); - $element_id = GETPOST('element_id', 'int', 2); +if (GETPOST('roworder', 'alpha', 3) && GETPOST('table_element_line', 'aZ09', 3) + && GETPOST('fk_element', 'aZ09', 3) && GETPOST('element_id', 'int', 3)) { + $roworder = GETPOST('roworder', 'alpha', 3); + $table_element_line = GETPOST('table_element_line', 'aZ09', 3); + $fk_element = GETPOST('fk_element', 'aZ09', 3); + $element_id = GETPOST('element_id', 'int', 3); dol_syslog("AjaxRow roworder=".$roworder." table_element_line=".$table_element_line." fk_element=".$fk_element." element_id=".$element_id, LOG_DEBUG); - // Make test on pemrission + // Make test on permission $perm = 0; if ($table_element_line == 'propaldet' && $user->rights->propal->creer) { $perm = 1; @@ -92,6 +95,10 @@ if (GETPOST('roworder', 'alpha', 2) && GETPOST('table_element_line', 'aZ09', 2) $perm = 1; } elseif ($table_element_line == 'facture_fourn_det' && $user->rights->fourn->facture->creer) { $perm = 1; + } elseif ($table_element_line == 'ecm_files' && $fk_element == 'fk_product' && (!empty($user->rights->produit->creer) || !empty($user->rights->service->creer))) { + $perm = 1; + } elseif ($table_element_line == 'ecm_files' && $fk_element == 'fk_ticket' && !empty($user->rights->ticket->write)) { + $perm = 1; } else { $tmparray = explode('_', $table_element_line); $tmpmodule = $tmparray[0]; $tmpobject = preg_replace('/line$/', '', $tmparray[1]); @@ -101,7 +108,10 @@ if (GETPOST('roworder', 'alpha', 2) && GETPOST('table_element_line', 'aZ09', 2) } if (! $perm) { + // We should not be here. If we are not allowed to reorder rows, feature should not be visible on script. + // If we are here, it is a hack attempt, so we report a warning. print 'Bad permission to modify position of lines for object in table '.$table_element_line; + dol_syslog('Bad permission to modify position of lines for object in table '.$table_element_line.', fk_element '.$fk_element, LOG_WARNING); accessforbidden('Bad permission to modify position of lines for object in table '.$table_element_line); } diff --git a/htdocs/core/lib/security.lib.php b/htdocs/core/lib/security.lib.php index 1440a02983f..de4d67b1647 100644 --- a/htdocs/core/lib/security.lib.php +++ b/htdocs/core/lib/security.lib.php @@ -350,7 +350,7 @@ function restrictedArea($user, $features, $objectid = 0, $tableandshare = '', $f // Check write permission from module (we need to know write permission to create but also to delete drafts record or to upload files) $createok = 1; $nbko = 0; - $wemustcheckpermissionforcreate = (GETPOST('sendit', 'alpha') || GETPOST('linkit', 'alpha') || GETPOST('action', 'aZ09') == 'create' || GETPOST('action', 'aZ09') == 'update'); + $wemustcheckpermissionforcreate = (GETPOST('sendit', 'alpha') || GETPOST('linkit', 'alpha') || GETPOST('action', 'aZ09') == 'create' || GETPOST('action', 'aZ09') == 'update') || GETPOST('roworder', 'alpha', 2); $wemustcheckpermissionfordeletedraft = ((GETPOST("action", "aZ09") == 'confirm_delete' && GETPOST("confirm", "aZ09") == 'yes') || GETPOST("action", "aZ09") == 'delete'); if ($wemustcheckpermissionforcreate || $wemustcheckpermissionfordeletedraft) { diff --git a/htdocs/modulebuilder/template/myobject_document.php b/htdocs/modulebuilder/template/myobject_document.php index 8598cc3dd18..e3fa6390e93 100644 --- a/htdocs/modulebuilder/template/myobject_document.php +++ b/htdocs/modulebuilder/template/myobject_document.php @@ -124,7 +124,7 @@ if ($id > 0 || !empty($ref)) { $upload_dir = $conf->mymodule->multidir_output[$object->entity ? $object->entity : $conf->entity]."/myobject/".get_exdir(0, 0, 0, 1, $object); } -$permissiontoadd = $user->rights->mymodule->myobject->write; // Used by the include of actions_addupdatedelete.inc.php +$permissiontoadd = $user->rights->mymodule->myobject->write; // Used by the include of actions_addupdatedelete.inc.php and actions_linkedfiles.inc.php // Security check (enable the most restrictive one) //if ($user->socid > 0) accessforbidden(); diff --git a/htdocs/product/document.php b/htdocs/product/document.php index af44521dc05..372e3c19bdc 100644 --- a/htdocs/product/document.php +++ b/htdocs/product/document.php @@ -113,7 +113,7 @@ if ($reshook < 0) { if (empty($reshook)) { // Delete line if product propal merge is linked to a file if (!empty($conf->global->PRODUIT_PDF_MERGE_PROPAL)) { - if ($action == 'confirm_deletefile' && $confirm == 'yes') { + if ($action == 'confirm_deletefile' && $confirm == 'yes' && $permissiontoadd) { //extract file name $urlfile = GETPOST('urlfile', 'alpha'); $filename = basename($urlfile); @@ -131,7 +131,7 @@ if (empty($reshook)) { include DOL_DOCUMENT_ROOT.'/core/actions_linkedfiles.inc.php'; } -if ($action == 'filemerge') { +if ($action == 'filemerge' && $permissiontoadd) { $is_refresh = GETPOST('refresh'); if (empty($is_refresh)) { $filetomerge_file_array = GETPOST('filetoadd'); diff --git a/htdocs/projet/document.php b/htdocs/projet/document.php index 3f9ec04d437..6bb905d6696 100644 --- a/htdocs/projet/document.php +++ b/htdocs/projet/document.php @@ -40,11 +40,6 @@ $ref = GETPOST('ref', 'alpha'); $mine = (GETPOST('mode', 'alpha') == 'mine' ? 1 : 0); //if (! $user->rights->projet->all->lire) $mine=1; // Special for projects -// Security check -$socid = 0; -//if ($user->socid > 0) $socid = $user->socid; // For external user, no check is done on company because readability is managed by public status of project and assignement. -$result = restrictedArea($user, 'projet', $id, 'projet&project'); - $object = new Project($db); include DOL_DOCUMENT_ROOT.'/core/actions_fetchobject.inc.php'; // Must be include, not include_once @@ -82,6 +77,11 @@ if (!$sortfield) { $sortfield = "name"; } +// Security check +$socid = 0; +//if ($user->socid > 0) $socid = $user->socid; // For external user, no check is done on company because readability is managed by public status of project and assignement. +$result = restrictedArea($user, 'projet', $id, 'projet&project'); + /* diff --git a/htdocs/societe/document.php b/htdocs/societe/document.php index 4ed26f67018..653069882e0 100644 --- a/htdocs/societe/document.php +++ b/htdocs/societe/document.php @@ -76,6 +76,7 @@ if ($id > 0 || !empty($ref)) { // Initialize technical object to manage hooks of page. Note that conf->hooks_modules contains array of hook context $hookmanager->initHooks(array('thirdpartydocument', 'globalcard')); +$permissiontoadd = $user->rights->societe->creer; // Used by the include of actions_addupdatedelete.inc.php and actions_lineupdown.inc.php // Security check if ($user->socid > 0) { diff --git a/htdocs/ticket/agenda.php b/htdocs/ticket/agenda.php index 9bf6cbc13c4..fee91e959aa 100644 --- a/htdocs/ticket/agenda.php +++ b/htdocs/ticket/agenda.php @@ -81,12 +81,9 @@ if (!$action) { // Security check $id = GETPOST("id", 'int'); $socid = 0; -//if ($user->socid > 0) $socid = $user->socid; // For external user, no check is done on company because readability is managed by public status of project and assignement. +if ($user->socid > 0) $socid = $user->socid; $result = restrictedArea($user, 'ticket', $id, ''); -if (!$user->rights->ticket->read) { - accessforbidden(); -} // restrict access for externals users if ($user->socid > 0 && ($object->fk_soc != $user->socid)) { accessforbidden(); diff --git a/htdocs/ticket/card.php b/htdocs/ticket/card.php index c4c637754b4..5e2ef1e804b 100644 --- a/htdocs/ticket/card.php +++ b/htdocs/ticket/card.php @@ -112,8 +112,8 @@ if ($id || $track_id || $ref) { $url_page_current = DOL_URL_ROOT.'/ticket/card.php'; // Security check - Protection if external user -//if ($user->socid > 0) accessforbidden(); -//if ($user->socid > 0) $socid = $user->socid; +$socid = 0; +if ($user->socid > 0) $socid = $user->socid; $result = restrictedArea($user, 'ticket', $object->id); $triggermodname = 'TICKET_MODIFY'; diff --git a/htdocs/ticket/contact.php b/htdocs/ticket/contact.php index 7e04dd12104..8d450ffb44b 100644 --- a/htdocs/ticket/contact.php +++ b/htdocs/ticket/contact.php @@ -50,11 +50,6 @@ $source = GETPOST('source', 'alpha'); $ligne = GETPOST('ligne', 'int'); $lineid = GETPOST('lineid', 'int'); -// Protection if external user -if ($user->socid > 0) { - $socid = $user->socid; - accessforbidden(); -} // Store current page url $url_page_current = dol_buildpath('/ticket/contact.php', 1); @@ -62,6 +57,24 @@ $url_page_current = dol_buildpath('/ticket/contact.php', 1); $object = new Ticket($db); +$permissiontoadd = $user->rights->ticket->write; + +// Security check +$id = GETPOST("id", 'int'); +$socid = 0; +if ($user->socid > 0) $socid = $user->socid; +$result = restrictedArea($user, 'ticket', $object->id, ''); + +// restrict access for externals users +if ($user->socid > 0 && ($object->fk_soc != $user->socid)) { + accessforbidden(); +} +// or for unauthorized internals users +if (!$user->socid && (!empty($conf->global->TICKET_LIMIT_VIEW_ASSIGNED_ONLY) && $object->fk_user_assign != $user->id) && !$user->rights->ticket->manage) { + accessforbidden(); +} + + /* * Actions */ diff --git a/htdocs/ticket/document.php b/htdocs/ticket/document.php index 5e4d80cd8be..8edd2787c44 100644 --- a/htdocs/ticket/document.php +++ b/htdocs/ticket/document.php @@ -43,11 +43,6 @@ $track_id = GETPOST('track_id', 'alpha'); $action = GETPOST('action', 'alpha'); $confirm = GETPOST('confirm', 'alpha'); -// Security check -if (!$user->rights->ticket->read) { - accessforbidden(); -} - // Get parameters $limit = GETPOST('limit', 'int') ? GETPOST('limit', 'int') : $conf->liste_limit; $sortfield = GETPOST("sortfield", 'alpha'); @@ -75,6 +70,21 @@ if ($result < 0) { $upload_dir = $conf->ticket->dir_output."/".dol_sanitizeFileName($object->ref); } +$permissiontoadd = $user->rights->ticket->write; + +// Security check - Protection if external user +$result = restrictedArea($user, 'ticket', $object->id); + +// restrict access for externals users +if ($user->socid > 0 && ($object->fk_soc != $user->socid)) { + accessforbidden(); +} +// or for unauthorized internals users +if (!$user->socid && ($conf->global->TICKET_LIMIT_VIEW_ASSIGNED_ONLY && $object->fk_user_assign != $user->id) && !$user->rights->ticket->manage) { + accessforbidden(); +} + + /* * Actions diff --git a/htdocs/ticket/messaging.php b/htdocs/ticket/messaging.php index c3e70def7da..68615424abf 100644 --- a/htdocs/ticket/messaging.php +++ b/htdocs/ticket/messaging.php @@ -76,16 +76,14 @@ if (!$action) { $action = 'view'; } +$permissiontoadd = $user->rights->ticket->write; // Security check $id = GETPOST("id", 'int'); $socid = 0; -//if ($user->socid > 0) $socid = $user->socid; // For external user, no check is done on company because readability is managed by public status of project and assignement. -$result = restrictedArea($user, 'ticket', $id, ''); +if ($user->socid > 0) $socid = $user->socid; +$result = restrictedArea($user, 'ticket', $object->id, ''); -if (!$user->rights->ticket->read) { - accessforbidden(); -} // restrict access for externals users if ($user->socid > 0 && ($object->fk_soc != $user->socid)) { accessforbidden(); @@ -96,7 +94,6 @@ if (!$user->socid && (!empty($conf->global->TICKET_LIMIT_VIEW_ASSIGNED_ONLY) && } - /* * Actions */