lainwir3d/dolibarr
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

FIX missing security test on payment page

Browse Source 
FIX sql error on group by on payment list
This commit is contained in:
Laurent Destailleur 2021-02-19 12:35:26 +01:00
parent 27818b68ae
commit 5340c30db3
10 changed files with 124 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
htdocs/compta/paiement/card.php
View File

@ -43,12 +43,20 @@ $action = GETPOST('action', 'aZ09');
$confirm = GETPOST('confirm', 'alpha');
$backtopage = GETPOST('backtopage', 'alpha');
$object = new Paiement($db);
// Load object
include DOL_DOCUMENT_ROOT.'/core/actions_fetchobject.inc.php'; // Must be include, not include_once.
$result = restrictedArea($user, $object->element, $object->id, 'paiement', '');
// Security check
if ($user->socid) $socid = $user->socid;
// TODO ajouter regle pour restreindre acces paiement
//$result = restrictedArea($user, 'facture', $id,'');
$object = new Paiement($db);
// Now check also permission on thirdparty of invoices of payments. Thirdparty were loaded by the fetch_object before based on first invoice.
// It should be enough because all payments are done on invoices of the same thirdparty.
if ($socid && $socid != $object->thirdparty->id) {
accessforbidden();
}
/*
@ -59,7 +67,6 @@ if ($action == 'setnote' && $user->rights->facture->paiement)
{
$db->begin();
$object->fetch($id);
$result = $object->update_note(GETPOST('note', 'restricthtml'));
if ($result > 0)
{
@ -75,7 +82,6 @@ if ($action == 'confirm_delete' && $confirm == 'yes' && $user->rights->facture->
{
$db->begin();
$object->fetch($id);
$result = $object->delete();
if ($result > 0)
{
@ -100,7 +106,6 @@ if ($action == 'confirm_validate' && $confirm == 'yes' && $user->rights->facture
{
$db->begin();
$object->fetch($id);
if ($object->validate($user) > 0)
{
$db->commit();
@ -134,7 +139,6 @@ if ($action == 'confirm_validate' && $confirm == 'yes' && $user->rights->facture
if ($action == 'setnum_paiement' && !empty($_POST['num_paiement']))
{
$object->fetch($id);
$res = $object->update_num($_POST['num_paiement']);
if ($res === 0)
{
@ -146,7 +150,6 @@ if ($action == 'setnum_paiement' && !empty($_POST['num_paiement']))
if ($action == 'setdatep' && !empty($_POST['datepday']))
{
$object->fetch($id);
$datepaye = dol_mktime(GETPOST('datephour', 'int'), GETPOST('datepmin', 'int'), GETPOST('datepsec', 'int'), GETPOST('datepmonth', 'int'), GETPOST('datepday', 'int'), GETPOST('datepyear', 'int'));
$res = $object->update_date($datepaye);
if ($res === 0)

3
htdocs/compta/paiement/class/paiement.class.php
View File

@ -1293,7 +1293,8 @@ class Paiement extends CommonObject
// phpcs:disable PEAR.NamingConventions.ValidFunctionName.ScopeNotCamelCaps
/**
* Load the third party of object, from id into this->thirdparty
* Load the third party of object, from id into this->thirdparty.
* For payments, take the thirdparty linked to the first invoice found. This is enough because payments are done on invoices of the same thirdparty.
*
* @param int $force_thirdparty_id Force thirdparty id
* @return int <0 if KO, >0 if OK

19
htdocs/compta/paiement/info.php
View File

@ -36,6 +36,23 @@ $ref = GETPOST('ref', 'alpha');
$action = GETPOST('action', 'aZ09');
$confirm = GETPOST('confirm', 'alpha');
$object = new Paiement($db);
// Load object
include DOL_DOCUMENT_ROOT.'/core/actions_fetchobject.inc.php'; // Must be include, not include_once.
$result = restrictedArea($user, $object->element, $object->id, 'paiement', '');
// Security check
if ($user->socid) $socid = $user->socid;
// Now check also permission on thirdparty of invoices of payments. Thirdparty were loaded by the fetch_object before based on first invoice.
// It should be enough because all payments are done on invoices of the same thirdparty.
if ($socid && $socid != $object->thirdparty->id) {
accessforbidden();
}
/*
* Actions
*/
@ -49,8 +66,6 @@ $confirm = GETPOST('confirm', 'alpha');
llxHeader('', $langs->trans("Payment"));
$object = new Paiement($db);
$object->fetch($id, $ref);
$object->info($object->id);
$head = payment_prepare_head($object);

6
htdocs/compta/paiement/rapport.php
View File

@ -30,9 +30,6 @@ require_once DOL_DOCUMENT_ROOT.'/core/lib/files.lib.php';
require_once DOL_DOCUMENT_ROOT.'/core/class/html.formfile.class.php';
require_once DOL_DOCUMENT_ROOT.'/core/class/html.formother.class.php';
// Security check
if (!$user->rights->facture->lire) accessforbidden();
$action = GETPOST('action', 'aZ09');
$socid = 0;
@ -48,6 +45,9 @@ if (!$user->rights->societe->client->voir || $socid) $dir .= '/private/'.$user->
$year = GETPOST('year', 'int');
if (!$year) { $year = date("Y"); }
// Security check
if (empty($user->rights->facture->lire)) accessforbidden();
/*
* Actions

7
htdocs/compta/paiement/tovalidate.php
View File

@ -27,10 +27,6 @@ require '../../main.inc.php';
// Load translation files required by the page
$langs->load("bills");
// Security check
if (!$user->rights->facture->lire)
accessforbidden();
$socid = 0;
if ($user->socid > 0)
{
@ -50,6 +46,9 @@ $pagenext = $page + 1;
if (!$sortorder) $sortorder = "DESC";
if (!$sortfield) $sortfield = "p.rowid";
// Security check
if (empty($user->rights->facture->lire)) accessforbidden();
/*
* Actions

59
htdocs/core/lib/security.lib.php
View File

@ -165,6 +165,7 @@ function dol_verifyHash($chain, $hash, $type = '0')
/**
* Check permissions of a user to show a page and an object. Check read permission.
* If GETPOST('action','aZ09') defined, we also check write and delete permission.
* This method check permission on module then call checkUserAccessToObject() for permission on object (according to entity and socid of user).
*
* @param User $user User to check
* @param string $features Features to check (it must be module $object->element. Examples: 'societe', 'contact', 'produit&service', 'produit|service', ...)
@ -175,20 +176,22 @@ function dol_verifyHash($chain, $hash, $type = '0')
* @param string $dbt_select Field name for select if not rowid. Not used if objectid is null (optional)
* @param int $isdraft 1=The object with id=$objectid is a draft
* @return int Always 1, die process if not allowed
* @see dol_check_secure_access_document()
* @see dol_check_secure_access_document(), checkUserAccessToObject()
*/
function restrictedArea($user, $features, $objectid = 0, $tableandshare = '', $feature2 = '', $dbt_keyfield = 'fk_soc', $dbt_select = 'rowid', $isdraft = 0)
{
global $db, $conf;
global $hookmanager;
//dol_syslog("functions.lib:restrictedArea $feature, $objectid, $dbtablename,$feature2,$dbt_socfield,$dbt_select");
//dol_syslog("functions.lib:restrictedArea $feature, $objectid, $dbtablename, $feature2, $dbt_socfield, $dbt_select, $isdraft");
//print "user_id=".$user->id.", features=".$features.", feature2=".$feature2.", objectid=".$objectid;
//print ", dbtablename=".$dbtablename.", dbt_socfield=".$dbt_keyfield.", dbt_select=".$dbt_select;
//print ", perm: ".$features."->".$feature2."=".($user->rights->$features->$feature2->lire)."<br>";
$parentfortableentity = '';
// Fix syntax of $features param
$originalfeatures = $features;
if ($features == 'facturerec') $features = 'facture';
if ($features == 'mo') $features = 'mrp';
if ($features == 'member') $features = 'adherent';
@ -198,7 +201,7 @@ function restrictedArea($user, $features, $objectid = 0, $tableandshare = '', $f
if ($features == 'product') $features = 'produit';
// Get more permissions checks from hooks
$parameters = array('features'=>$features, 'objectid'=>$objectid, 'idtype'=>$dbt_select);
$parameters = array('features'=>$features, 'originalfeatures'=>$originalfeatures, 'objectid'=>$objectid, 'dbt_select'=>$dbt_select, 'idtype'=>$dbt_select, 'isdraft'=>$isdraft);
$reshook = $hookmanager->executeHooks('restrictedArea', $parameters);
if (isset($hookmanager->resArray['result'])) {
@ -218,11 +221,6 @@ function restrictedArea($user, $features, $objectid = 0, $tableandshare = '', $f
// More subfeatures to check
if (!empty($feature2)) $feature2 = explode("|", $feature2);
// More parameters
$params = explode('&', $tableandshare);
$dbtablename = (!empty($params[0]) ? $params[0] : '');
$sharedelement = (!empty($params[1]) ? $params[1] : $dbtablename);
$listofmodules = explode(',', $conf->global->MAIN_MODULES_FOR_EXTERNAL);
// Check read permission from module
@ -247,6 +245,10 @@ function restrictedArea($user, $features, $objectid = 0, $tableandshare = '', $f
if (!$user->rights->banque->cheque) { $readok = 0; $nbko++; }
} elseif ($feature == 'projet') {
if (!$user->rights->projet->lire && !$user->rights->projet->all->lire) { $readok = 0; $nbko++; }
} elseif ($feature == 'payment') {
if (!$user->rights->facture->lire) { $readok = 0; $nbko++; }
} elseif ($feature == 'payment_supplier') {
if (!$user->rights->fournisseur->facture->lire) { $readok = 0; $nbko++; }
} elseif (!empty($feature2)) { // This is for permissions on 2 levels
$tmpreadok = 1;
foreach ($feature2 as $subfeature) {
@ -426,6 +428,10 @@ function checkUserAccessToObject($user, $featuresarray, $objectid = 0, $tableand
{
global $db, $conf;
//dol_syslog("functions.lib:restrictedArea $feature, $objectid, $dbtablename, $feature2, $dbt_socfield, $dbt_select, $isdraft");
//print "user_id=".$user->id.", features=".join(',', $featuresarray).", feature2=".$feature2.", objectid=".$objectid;
//print ", tableandshare=".$tableandshare.", dbt_socfield=".$dbt_keyfield.", dbt_select=".$dbt_select."<br>";
// More parameters
$params = explode('&', $tableandshare);
$dbtablename = (!empty($params[0]) ? $params[0] : '');
@ -440,13 +446,13 @@ function checkUserAccessToObject($user, $featuresarray, $objectid = 0, $tableand
if ($feature == 'project') $feature = 'projet';
if ($feature == 'task') $feature = 'projet_task';
$check = array('adherent', 'banque', 'bom', 'don', 'mrp', 'user', 'usergroup', 'product', 'produit', 'service', 'produit|service', 'categorie', 'resource', 'expensereport', 'holiday', 'website'); // Test on entity only (Objects with no link to company)
$check = array('adherent', 'banque', 'bom', 'don', 'mrp', 'user', 'usergroup', 'payment', 'payment_supplier', 'product', 'produit', 'service', 'produit|service', 'categorie', 'resource', 'expensereport', 'holiday', 'website'); // Test on entity only (Objects with no link to company)
$checksoc = array('societe'); // Test for societe object
$checkother = array('contact', 'agenda'); // Test on entity and link to third party. Allowed if link is empty (Ex: contacts...).
$checkother = array('contact', 'agenda'); // Test on entity + link to third party on field $dbt_keyfield. Allowed if link is empty (Ex: contacts...).
$checkproject = array('projet', 'project'); // Test for project object
$checktask = array('projet_task'); // Test for task object
$nocheck = array('barcode', 'stock'); // No test
//$checkdefault = 'all other not already defined'; // Test on entity and link to third party. Not allowed if link is empty (Ex: invoice, orders...).
//$checkdefault = 'all other not already defined'; // Test on entity + link to third party on field $dbt_keyfield. Not allowed if link is empty (Ex: invoice, orders...).
// If dbtablename not defined, we use same name for table than module name
if (empty($dbtablename))
@ -455,17 +461,14 @@ function checkUserAccessToObject($user, $featuresarray, $objectid = 0, $tableand
$sharedelement = (!empty($params[1]) ? $params[1] : $dbtablename); // We change dbtablename, so we set sharedelement too.
}
// Check permission for object with entity
// Check permission for object on entity only
if (in_array($feature, $check))
{
$sql = "SELECT COUNT(dbt.".$dbt_select.") as nb";
$sql .= " FROM ".MAIN_DB_PREFIX.$dbtablename." as dbt";
if (($feature == 'user' || $feature == 'usergroup') && !empty($conf->multicompany->enabled))
{
if (!empty($conf->global->MULTICOMPANY_TRANSVERSE_MODE))
{
if ($conf->entity == 1 && $user->admin && !$user->entity)
{
if (($feature == 'user' || $feature == 'usergroup') && !empty($conf->multicompany->enabled)) { // Special for multicompany
if (!empty($conf->global->MULTICOMPANY_TRANSVERSE_MODE)) {
if ($conf->entity == 1 && $user->admin && !$user->entity) {
$sql .= " WHERE dbt.".$dbt_select." IN (".$objectid.")";
$sql .= " AND dbt.entity IS NOT NULL";
} else {
@ -490,15 +493,12 @@ function checkUserAccessToObject($user, $featuresarray, $objectid = 0, $tableand
$sql .= " AND dbt.entity IN (".getEntity($sharedelement, 1).")";
}
}
} elseif (in_array($feature, $checksoc)) // We check feature = checksoc
{
// If external user: Check permission for external users
if ($user->socid > 0)
{
} elseif (in_array($feature, $checksoc)) { // We check feature = checksoc
if ($user->socid > 0) {
// If external user: Check permission for external users
if ($user->socid <> $objectid) return false;
} // If internal user: Check permission for internal users that are restricted on their objects
elseif (!empty($conf->societe->enabled) && ($user->rights->societe->lire && !$user->rights->societe->client->voir))
{
} elseif (!empty($conf->societe->enabled) && ($user->rights->societe->lire && !$user->rights->societe->client->voir)) {
// If internal user: Check permission for internal users that are restricted on their objects
$sql = "SELECT COUNT(sc.fk_soc) as nb";
$sql .= " FROM (".MAIN_DB_PREFIX."societe_commerciaux as sc";
$sql .= ", ".MAIN_DB_PREFIX."societe as s)";
@ -506,15 +506,14 @@ function checkUserAccessToObject($user, $featuresarray, $objectid = 0, $tableand
$sql .= " AND sc.fk_user = ".$user->id;
$sql .= " AND sc.fk_soc = s.rowid";
$sql .= " AND s.entity IN (".getEntity($sharedelement, 1).")";
} // If multicompany and internal users with all permissions, check user is in correct entity
elseif (!empty($conf->multicompany->enabled))
{
} elseif (!empty($conf->multicompany->enabled)) {
// If multicompany and internal users with all permissions, check user is in correct entity
$sql = "SELECT COUNT(s.rowid) as nb";
$sql .= " FROM ".MAIN_DB_PREFIX."societe as s";
$sql .= " WHERE s.rowid IN (".$objectid.")";
$sql .= " AND s.entity IN (".getEntity($sharedelement, 1).")";
}
} elseif (in_array($feature, $checkother)) // Test on entity and link to societe. Allowed if link is empty (Ex: contacts...).
} elseif (in_array($feature, $checkother)) // Test on entity + link to thirdparty. Allowed if link is empty (Ex: contacts...).
{
// If external user: Check permission for external users
if ($user->socid > 0)

47
htdocs/fourn/paiement/card.php
View File

@ -40,10 +40,19 @@ $confirm = GETPOST('confirm', 'alpha');
$object = new PaiementFourn($db);
// PDF
$hidedetails = (GETPOST('hidedetails', 'int') ? GETPOST('hidedetails', 'int') : (!empty($conf->global->MAIN_GENERATE_DOCUMENTS_HIDE_DETAILS) ? 1 : 0));
$hidedesc = (GETPOST('hidedesc', 'int') ? GETPOST('hidedesc', 'int') : (!empty($conf->global->MAIN_GENERATE_DOCUMENTS_HIDE_DESC) ? 1 : 0));
$hideref = (GETPOST('hideref', 'int') ? GETPOST('hideref', 'int') : (!empty($conf->global->MAIN_GENERATE_DOCUMENTS_HIDE_REF) ? 1 : 0));
// Load object
include DOL_DOCUMENT_ROOT.'/core/actions_fetchobject.inc.php'; // Must be include, not include_once.
$result = restrictedArea($user, $object->element, $object->id, 'paiementfourn', '');
// Security check
if ($user->socid) $socid = $user->socid;
// Now check also permission on thirdparty of invoices of payments. Thirdparty were loaded by the fetch_object before based on first invoice.
// It should be enough because all payments are done on invoices of the same thirdparty.
if ($socid && $socid != $object->thirdparty->id) {
accessforbidden();
}
/*
* Actions
@ -177,41 +186,46 @@ if ($result > 0)
print '<table class="border centpercent">';
/*print '<tr>';
print '<td width="20%" colspan="2">'.$langs->trans('Ref').'</td><td colspan="3">';
print '<td width="20%">'.$langs->trans('Ref').'</td><td>';
print $form->showrefnav($object,'id','',1,'rowid','ref');
print '</td></tr>';*/
// Date of payment
print '<tr><td class="titlefield" colspan="2">'.$form->editfieldkey("Date", 'datep', $object->date, $object, $object->statut == 0 && $user->rights->fournisseur->facture->creer).'</td><td colspan="3">';
print '<tr><td class="titlefield">'.$form->editfieldkey("Date", 'datep', $object->date, $object, $object->statut == 0 && $user->rights->fournisseur->facture->creer).'</td>';
print '<td>';
print $form->editfieldval("Date", 'datep', $object->date, $object, $object->statut == 0 && $user->rights->fournisseur->facture->creer, 'datehourpicker', '', null, $langs->trans('PaymentDateUpdateSucceeded'));
print '</td></tr>';
// Payment mode
$labeltype = $langs->trans("PaymentType".$object->type_code) != ("PaymentType".$object->type_code) ? $langs->trans("PaymentType".$object->type_code) : $object->type_label;
print '<tr><td colspan="2">'.$langs->trans('PaymentMode').'</td><td colspan="3">'.$labeltype;
print '<tr><td>'.$langs->trans('PaymentMode').'</td>';
print '<td>'.$labeltype;
print $object->num_payment ? ' - '.$object->num_payment : '';
print '</td></tr>';
// Payment numero
/* TODO Add field num_payment into payment table and save it
print '<tr><td colspan="2">'.$form->editfieldkey("Numero",'num_paiement',$object->num_paiement,$object,$object->statut == 0 && $user->rights->fournisseur->facture->creer).'</td><td colspan="3">';
print '<tr><td>'.$form->editfieldkey("Numero",'num_paiement',$object->num_paiement,$object,$object->statut == 0 && $user->rights->fournisseur->facture->creer).'</td>';
print '<td>';
print $form->editfieldval("Numero",'num_paiement',$object->num_paiement,$object,$object->statut == 0 && $user->rights->fournisseur->facture->creer,'string','',null,$langs->trans('PaymentNumberUpdateSucceeded'));
print '</td></tr>';
*/
// Amount
print '<tr><td colspan="2">'.$langs->trans('Amount').'</td><td colspan="3">'.price($object->amount, '', $langs, 0, 0, -1, $conf->currency).'</td></tr>';
print '<tr><td>'.$langs->trans('Amount').'</td>';
print '<td>'.price($object->amount, '', $langs, 0, 0, -1, $conf->currency).'</td></tr>';
if (!empty($conf->global->BILL_ADD_PAYMENT_VALIDATION))
{
print '<tr><td colspan="2">'.$langs->trans('Status').'</td><td colspan="3">'.$object->getLibStatut(4).'</td></tr>';
print '<tr><td>'.$langs->trans('Status').'</td>';
print '<td>'.$object->getLibStatut(4).'</td></tr>';
}
$allow_delete = 1;
// Bank account
if (!empty($conf->banque->enabled))
{
if ($object->bank_account)
if ($object->fk_account)
{
$bankline = new AccountLine($db);
$bankline->fetch($object->bank_line);
@ -222,8 +236,8 @@ if ($result > 0)
}
print '<tr>';
print '<td colspan="2">'.$langs->trans('BankAccount').'</td>';
print '<td colspan="3">';
print '<td>'.$langs->trans('BankAccount').'</td>';
print '<td>';
$accountstatic = new Account($db);
$accountstatic->fetch($bankline->fk_account);
print $accountstatic->getNomUrl(1);
@ -231,8 +245,8 @@ if ($result > 0)
print '</tr>';
print '<tr>';
print '<td colspan="2">'.$langs->trans('BankTransactionLine').'</td>';
print '<td colspan="3">';
print '<td>'.$langs->trans('BankTransactionLine').'</td>';
print '<td>';
print $bankline->getNomUrl(1, 0, 'showconciliated');
print '</td>';
print '</tr>';
@ -240,7 +254,8 @@ if ($result > 0)
}
// Note
print '<tr><td colspan="2">'.$form->editfieldkey("Note", 'note', $object->note, $object, $user->rights->fournisseur->facture->creer).'</td><td colspan="3">';
print '<tr><td>'.$form->editfieldkey("Comments", 'note', $object->note, $object, $user->rights->fournisseur->facture->creer).'</td>';
print '<td>';
print $form->editfieldval("Note", 'note', $object->note, $object, $user->rights->fournisseur->facture->creer, 'textarea');
print '</td></tr>';

26
htdocs/fourn/paiement/info.php
View File

@ -33,8 +33,26 @@ $langs->loadLangs(array("bills", "suppliers", "companies"));
$id = GETPOST('id', 'int');
$object = new PaiementFourn($db);
$object->fetch($id);
$object->info($id);
// Load object
include DOL_DOCUMENT_ROOT.'/core/actions_fetchobject.inc.php'; // Must be include, not include_once.
$result = restrictedArea($user, $object->element, $object->id, 'paiementfourn', '');
// Security check
if ($user->socid) $socid = $user->socid;
// Now check also permission on thirdparty of invoices of payments. Thirdparty were loaded by the fetch_object before based on first invoice.
// It should be enough because all payments are done on invoices of the same thirdparty.
if ($socid && $socid != $object->thirdparty->id) {
accessforbidden();
}
/*
* Actions
*/
// None
/*
@ -43,10 +61,14 @@ $object->info($id);
llxHeader();
$object->info($id);
$head = payment_supplier_prepare_head($object);
print dol_get_fiche_head($head, 'info', $langs->trans("SupplierPayment"), 0, 'payment');
$linkback = '<a href="'.DOL_URL_ROOT.'/fourn/paiement/list.php?restore_lastsearch_values=1">'.$langs->trans("BackToList").'</a>';
dol_banner_tab($object, 'id', $linkback, -1, 'rowid', 'ref');
print dol_get_fiche_end();

2
htdocs/fourn/paiement/list.php
View File

@ -178,7 +178,7 @@ if ($search_all) $sql .= natural_search(array_keys($fieldstosearchall), $search_
// Add where from extra fields
include DOL_DOCUMENT_ROOT.'/core/tpl/extrafields_list_search_sql.tpl.php';
$sql .= ' GROUP BY p.rowid, p.datep, p.amount, p.num_paiement, s.rowid, s.nom, c.code, c.libelle, ba.rowid, ba.label';
$sql .= ' GROUP BY p.rowid, p.ref, p.datep, p.amount, p.num_paiement, s.rowid, s.nom, s.email, c.code, c.libelle, ba.rowid, ba.label';
if (!$user->rights->societe->client->voir) $sql .= ', sc.fk_soc, sc.fk_user';
$sql .= $db->order($sortfield, $sortorder);

4
htdocs/modulebuilder/template/myobject_card.php
View File

@ -114,9 +114,9 @@ $upload_dir = $conf->mymodule->multidir_output[isset($object->entity) ? $object-
//if ($user->socid > 0) accessforbidden();
//if ($user->socid > 0) $socid = $user->socid;
//$isdraft = (($object->statut == $object::STATUS_DRAFT) ? 1 : 0);
//$result = restrictedArea($user, 'mymodule', $object->id, '', '', 'fk_soc', 'rowid', $isdraft);
//$result = restrictedArea($user, $object->element, $object->id, '', '', 'fk_soc', 'rowid', $isdraft);
//if (!$permissiontoread) accessforbidden();
//if (empty($permissiontoread)) accessforbidden();
/*