Merge pull request #21521 from hregis/fix_security_breach

FIX security breach if we have same ref number in multiple entities
This commit is contained in:
Laurent Destailleur 2022-07-19 00:53:05 +02:00 committed by GitHub
commit e3a4b5f12e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 35 additions and 38 deletions

View File

@ -1445,12 +1445,13 @@ class Propal extends CommonObject
/**
* Load a proposal from database. Get also lines.
*
* @param int $rowid id of object to load
* @param string $ref Ref of proposal
* @param string $ref_ext Ref ext of proposal
* @return int >0 if OK, <0 if KO
* @param int $rowid id of object to load
* @param string $ref Ref of proposal
* @param string $ref_ext Ref ext of proposal
* @param int $forceentity Entity id to force
* @return int >0 if OK, <0 if KO
*/
public function fetch($rowid, $ref = '', $ref_ext = '')
public function fetch($rowid, $ref = '', $ref_ext = '', $forceentity = 0)
{
$sql = "SELECT p.rowid, p.ref, p.entity, p.remise, p.remise_percent, p.remise_absolue, p.fk_soc";
$sql .= ", p.total_ttc, p.total_tva, p.localtax1, p.localtax2, p.total_ht";
@ -1489,10 +1490,15 @@ class Propal extends CommonObject
$sql .= ' LEFT JOIN '.MAIN_DB_PREFIX.'c_input_reason as dr ON p.fk_input_reason = dr.rowid';
$sql .= ' LEFT JOIN '.MAIN_DB_PREFIX.'c_incoterms as i ON p.fk_incoterms = i.rowid';
if ($ref) {
$sql .= " WHERE p.entity IN (".getEntity('propal').")"; // Dont't use entity if you use rowid
if (!empty($ref)) {
if (!empty($forceentity)) {
$sql .= " WHERE p.entity = ".(int) $forceentity; // Check only the current entity because we may have the same reference in several entities
} else {
$sql .= " WHERE p.entity IN (".getEntity('propal').")";
}
$sql .= " AND p.ref='".$this->db->escape($ref)."'";
} else {
// Dont't use entity if you use rowid
$sql .= " WHERE p.rowid = ".((int) $rowid);
}

View File

@ -26,7 +26,7 @@
*/
function showOnlineSignatureUrl($type, $ref)
{
global $conf, $langs;
global $langs;
// Load translation files required by the page
$langs->loadLangs(array("payment", "paybox"));
@ -59,7 +59,8 @@ function showOnlineSignatureUrl($type, $ref)
*/
function getOnlineSignatureUrl($mode, $type, $ref = '', $localorexternal = 1)
{
global $conf, $db, $langs, $dolibarr_main_url_root;
global $conf, $dolibarr_main_url_root;
global $object;
$ref = str_replace(' ', '', $ref);
$out = '';
@ -90,7 +91,7 @@ function getOnlineSignatureUrl($mode, $type, $ref = '', $localorexternal = 1)
if ($mode == 1) {
$out .= "hash('".$securekeyseed."' + '".$type."' + proposal_ref)";
} else {
$out .= '&securekey='.dol_hash($securekeyseed.$type.$ref, '0');
$out .= '&securekey='.dol_hash($securekeyseed.$type.$ref.$object->entity, '0');
}
/*
if ($mode == 1) {
@ -120,7 +121,7 @@ function getOnlineSignatureUrl($mode, $type, $ref = '', $localorexternal = 1)
// For multicompany
if (!empty($out) && !empty($conf->multicompany->enabled)) {
$out .= "&entity=".$conf->entity; // Check the entity because we may have the same reference in several entities
$out .= "&entity=".$object->entity; // Check the entity of object because we may have the same reference in several entities
}
return $out;

View File

@ -126,7 +126,7 @@ $creditor = $mysoc->name;
$type = $source;
if ($source == 'proposal') {
$object = new Propal($db);
$object->fetch(0, $ref);
$object->fetch(0, $ref, '', $entity);
} else {
accessforbidden('Bad value for source');
exit;
@ -139,7 +139,7 @@ if ($source == 'proposal') {
$securekeyseed = $conf->global->PROPOSAL_ONLINE_SIGNATURE_SECURITY_TOKEN;
}
if (!dol_verifyHash($securekeyseed.$type.$ref, $SECUREKEY, '0')) {
if (!dol_verifyHash($securekeyseed.$type.$ref.$object->entity, $SECUREKEY, '0')) {
http_response_code(403);
print 'Bad value for securitykey. Value provided '.dol_escape_htmltag($SECUREKEY).' does not match expected value for ref='.dol_escape_htmltag($ref);
exit(-1);
@ -288,18 +288,8 @@ $error = 0;
// Signature on commercial proposal
if ($source == 'proposal') {
$found = true;
$langs->load("proposal");
require_once DOL_DOCUMENT_ROOT.'/comm/propal/class/propal.class.php';
$proposal = new Propal($db);
$result = $proposal->fetch('', $ref);
if ($result <= 0) {
$mesg = $proposal->error;
$error++;
} else {
$result = $proposal->fetch_thirdparty($proposal->socid);
}
$result = $object->fetch_thirdparty($object->socid);
// Creditor
@ -315,39 +305,39 @@ if ($source == 'proposal') {
print '<tr class="CTableRow2"><td class="CTableRow2">'.$langs->trans("ThirdParty");
print '</td><td class="CTableRow2">';
print img_picto('', 'company', 'class="pictofixedwidth"');
print '<b>'.$proposal->thirdparty->name.'</b>';
print '<b>'.$object->thirdparty->name.'</b>';
print '</td></tr>'."\n";
// Amount
print '<tr class="CTableRow2"><td class="CTableRow2">'.$langs->trans("Amount");
print '</td><td class="CTableRow2">';
print '<b>'.price($proposal->total_ttc, 0, $langs, 1, -1, -1, $conf->currency).'</b>';
print '<b>'.price($object->total_ttc, 0, $langs, 1, -1, -1, $conf->currency).'</b>';
print '</td></tr>'."\n";
// Object
$text = '<b>'.$langs->trans("SignatureProposalRef", $proposal->ref).'</b>';
$text = '<b>'.$langs->trans("SignatureProposalRef", $object->ref).'</b>';
print '<tr class="CTableRow2"><td class="CTableRow2 tdtop">'.$langs->trans("Designation");
print '</td><td class="CTableRow2">'.$text;
if ($proposal->status == $proposal::STATUS_VALIDATED) {
$directdownloadlink = $proposal->getLastMainDocLink('proposal');
if ($object->status == $object::STATUS_VALIDATED) {
$directdownloadlink = $object->getLastMainDocLink('proposal');
if ($directdownloadlink) {
print '<br><a href="'.$directdownloadlink.'">';
print img_mime($proposal->last_main_doc, '');
print img_mime($object->last_main_doc, '');
print $langs->trans("DownloadDocument").'</a>';
}
} else {
$last_main_doc_file = $proposal->last_main_doc;
$last_main_doc_file = $object->last_main_doc;
if ($proposal->status == $proposal::STATUS_NOTSIGNED) {
$directdownloadlink = $proposal->getLastMainDocLink('proposal');
if ($object->status == $object::STATUS_NOTSIGNED) {
$directdownloadlink = $object->getLastMainDocLink('proposal');
if ($directdownloadlink) {
print '<br><a href="'.$directdownloadlink.'">';
print img_mime($proposal->last_main_doc, '');
print img_mime($object->last_main_doc, '');
print $langs->trans("DownloadDocument").'</a>';
}
} elseif ($proposal->status == $proposal::STATUS_SIGNED || $proposal->status == $proposal::STATUS_BILLED) {
} elseif ($object->status == $object::STATUS_SIGNED || $object->status == $object::STATUS_BILLED) {
if (preg_match('/_signed-(\d+)/', $last_main_doc_file)) { // If the last main doc has been signed
$last_main_doc_file_not_signed = preg_replace('/_signed-(\d+)/', '', $last_main_doc_file);
@ -355,10 +345,10 @@ if ($source == 'proposal') {
$datefilenotsigned = dol_filemtime($last_main_doc_file_not_signed);
if (empty($datefilenotsigned) || $datefilesigned > $datefilenotsigned) {
$directdownloadlink = $proposal->getLastMainDocLink('proposal');
$directdownloadlink = $object->getLastMainDocLink('proposal');
if ($directdownloadlink) {
print '<br><a href="'.$directdownloadlink.'">';
print img_mime($proposal->last_main_doc, '');
print img_mime($object->last_main_doc, '');
print $langs->trans("DownloadDocument").'</a>';
}
}
@ -367,7 +357,7 @@ if ($source == 'proposal') {
}
print '<input type="hidden" name="source" value="'.GETPOST("source", 'alpha').'">';
print '<input type="hidden" name="ref" value="'.$proposal->ref.'">';
print '<input type="hidden" name="ref" value="'.$object->ref.'">';
print '</td></tr>'."\n";
// TODO Add link to download PDF (similar code than for invoice)