From 4cacca413e32659c94482b5bb9cb376ab2362bdf Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Mon, 29 Mar 2021 14:11:51 +0200 Subject: [PATCH] FIX #yogosha5757 --- htdocs/core/lib/functions.lib.php | 15 +++++---- htdocs/core/lib/loan.lib.php | 33 +++++++++++-------- htdocs/expensereport/ajax/ajaxik.php | 11 +++---- .../class/expensereport_ik.class.php | 9 ++--- htdocs/loan/calcmens.php | 10 +++--- test/phpunit/SecurityTest.php | 5 +++ 6 files changed, 48 insertions(+), 35 deletions(-) diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index 403aa59e66a..d9c0685b96b 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -746,12 +746,14 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = case 'alpha': // No html and no ../ and " case 'alphanohtml': // Recommended for most scalar parameters and search parameters if (!is_array($out)) { - $out = dol_string_nohtmltag($out, 0); - // '"' is dangerous because param in url can close the href= or src= and add javascript functions. - // '../' is dangerous because it allows dir transversals $out = trim($out); do { $oldstringtoclean = $out; + // Remove html tags + $out = dol_string_nohtmltag($out, 0); + // Remove also other dangerous string sequences + // '"' is dangerous because param in url can close the href= or src= and add javascript functions. + // '../' is dangerous because it allows dir transversals // Note &, '&', '&'... is a simple char like '&' alone but there is no reason to accept such way to encode input data. $out = str_ireplace(array('&', '&', '&', '"', '"', '"', '"', '"', '/', '/', '/', '../'), '', $out); } while ($oldstringtoclean != $out); @@ -760,12 +762,13 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options = break; case 'alphawithlgt': // No " and no ../ but we keep balanced < > tags with no special chars inside. Can be used for email string like "Name " if (!is_array($out)) { - $out = dol_html_entity_decode($out, ENT_COMPAT | ENT_HTML5, 'UTF-8'); - // '"' is dangerous because param in url can close the href= or src= and add javascript functions. - // '../' is dangerous because it allows dir transversals $out = trim($out); do { $oldstringtoclean = $out; + // Remove html tags + $out = dol_html_entity_decode($out, ENT_COMPAT | ENT_HTML5, 'UTF-8'); + // '"' is dangerous because param in url can close the href= or src= and add javascript functions. + // '../' is dangerous because it allows dir transversals // Note &, '&', '&'... is a simple char like '&' alone but there is no reason to accept such way to encode input data. $out = str_ireplace(array('&', '&', '&', '"', '"', '"', '"', '"', '/', '/', '/', '../'), '', $out); } while ($oldstringtoclean != $out); diff --git a/htdocs/core/lib/loan.lib.php b/htdocs/core/lib/loan.lib.php index c86d0ecce13..4692c0b938b 100644 --- a/htdocs/core/lib/loan.lib.php +++ b/htdocs/core/lib/loan.lib.php @@ -90,20 +90,27 @@ function loan_prepare_head($object) /** * Calculate remaining loan mensuality and interests * - * @param int $mens Value of this mensuality (interests include, set 0 if we don't paid interests for this mensuality) - * @param float $capital Remaining capital for this mensuality - * @param float $rate Loan rate - * @param int $echance Actual loan term - * @param int $nbterm Total number of term for this loan - * @return array Array with remaining capital, interest, and mensuality for each remaining terms + * @param float $mens Value of this mensuality (interests include, set 0 if we don't paid interests for this mensuality) + * @param float $capital Remaining capital for this mensuality + * @param float $rate Loan rate + * @param int $numactualloadterm Actual loan term + * @param int $nbterm Total number of term for this loan + * @return array Array with remaining capital, interest, and mensuality for each remaining terms */ -function loanCalcMonthlyPayment($mens, $capital, $rate, $echance, $nbterm) +function loanCalcMonthlyPayment($mens, $capital, $rate, $numactualloadterm, $nbterm) { global $conf, $db; require_once DOL_DOCUMENT_ROOT.'/loan/class/loanschedule.class.php'; $object = new LoanSchedule($db); $output = array(); + // Sanitize data in case of + $mens = price2num($mens); + $capital = price2num($capital); + $rate = price2num($rate); + $numactualloadterm = ((int) $numactualloadterm); + $nbterm = ((int) $nbterm); + // If mensuality is 0 we don't pay interests and remaining capital not modified if ($mens == 0) { $int = 0; @@ -113,18 +120,18 @@ function loanCalcMonthlyPayment($mens, $capital, $rate, $echance, $nbterm) $int = round($int, 2, PHP_ROUND_HALF_UP); $cap_rest = round($capital - ($mens - $int), 2, PHP_ROUND_HALF_UP); } - $output[$echance] = array('cap_rest'=>$cap_rest, 'cap_rest_str'=>price($cap_rest, 0, '', 1, -1, -1, $conf->currency), 'interet'=>$int, 'interet_str'=>price($int, 0, '', 1, -1, -1, $conf->currency), 'mens'=>$mens); + $output[$numactualloadterm] = array('cap_rest'=>$cap_rest, 'cap_rest_str'=>price($cap_rest, 0, '', 1, -1, -1, $conf->currency), 'interet'=>$int, 'interet_str'=>price($int, 0, '', 1, -1, -1, $conf->currency), 'mens'=>$mens); - $echance++; + $numactualloadterm++; $capital = $cap_rest; - while ($echance <= $nbterm) { - $mens = round($object->calcMonthlyPayments($capital, $rate, $nbterm - $echance + 1), 2, PHP_ROUND_HALF_UP); + while ($numactualloadterm <= $nbterm) { + $mens = round($object->calcMonthlyPayments($capital, $rate, $nbterm - $numactualloadterm + 1), 2, PHP_ROUND_HALF_UP); $int = ($capital * ($rate / 12)); $int = round($int, 2, PHP_ROUND_HALF_UP); $cap_rest = round($capital - ($mens - $int), 2, PHP_ROUND_HALF_UP); - $output[$echance] = array( + $output[$numactualloadterm] = array( 'cap_rest' => $cap_rest, 'cap_rest_str' => price($cap_rest, 0, '', 1, -1, -1, $conf->currency), 'interet' => $int, @@ -133,7 +140,7 @@ function loanCalcMonthlyPayment($mens, $capital, $rate, $echance, $nbterm) ); $capital = $cap_rest; - $echance++; + $numactualloadterm++; } return $output; diff --git a/htdocs/expensereport/ajax/ajaxik.php b/htdocs/expensereport/ajax/ajaxik.php index 72c38959ef8..6b64a28d50c 100644 --- a/htdocs/expensereport/ajax/ajaxik.php +++ b/htdocs/expensereport/ajax/ajaxik.php @@ -49,18 +49,15 @@ require_once DOL_DOCUMENT_ROOT.'/expensereport/class/expensereport_ik.class.php' // Load translation files required by the page $langs->loadlangs(array('errors', 'trips')); + /* * View */ top_httphead(); - -dol_syslog(join(',', $_POST)); - -$fk_expense = GETPOST('fk_expense'); -$fk_c_exp_tax_cat = GETPOST('fk_c_exp_tax_cat'); - +$fk_expense = GETPOST('fk_expense', 'int'); +$fk_c_exp_tax_cat = GETPOST('fk_c_exp_tax_cat', 'int'); if (empty($fk_expense) || $fk_expense < 0) { echo json_encode(array('error' => $langs->transnoentitiesnoconv('ErrorBadValueForParameter', $fk_expense, 'fk_expense'))); @@ -82,7 +79,7 @@ if (empty($fk_expense) || $fk_expense < 0) { echo json_encode(array('error' => $langs->transnoentitiesnoconv('ErrorRecordNotFound'), 'range' => $range)); } else { $ikoffset = price($range->ikoffset, 0, $langs, 1, -1, -1, $conf->currency); - echo json_encode(array('up' => $range->coef, 'ikoffset' => $range->ikoffset, 'title' => $langs->transnoentitiesnoconv('ExpenseRangeOffset', $offset), 'comment' => 'offset should be apply on addline or updateline')); + echo json_encode(array('up' => $range->coef, 'ikoffset' => $range->ikoffset, 'title' => $langs->transnoentitiesnoconv('ExpenseRangeOffset', $ikoffset), 'comment' => 'offset should be applied on addline or updateline')); } } } diff --git a/htdocs/expensereport/class/expensereport_ik.class.php b/htdocs/expensereport/class/expensereport_ik.class.php index 18a0503cdd7..6c6623c30c7 100644 --- a/htdocs/expensereport/class/expensereport_ik.class.php +++ b/htdocs/expensereport/class/expensereport_ik.class.php @@ -137,7 +137,7 @@ class ExpenseReportIk extends CoreObject * @param int $fk_c_exp_tax_cat category * @return boolean|array */ - public static function getRangeByUser(User $userauthor, $fk_c_exp_tax_cat) + public static function getRangeByUser(User $userauthor, int $fk_c_exp_tax_cat) { $default_range = (int) $userauthor->default_range; // if not defined, then 0 $ranges = self::getRangesByCategory($fk_c_exp_tax_cat); @@ -157,23 +157,24 @@ class ExpenseReportIk extends CoreObject * @param int $active active * @return array */ - public static function getRangesByCategory($fk_c_exp_tax_cat, $active = 1) + public static function getRangesByCategory(int $fk_c_exp_tax_cat, $active = 1) { global $db; $ranges = array(); + dol_syslog(get_called_class().'::getRangesByCategory for fk_c_exp_tax_cat='.$fk_c_exp_tax_cat, LOG_DEBUG); + $sql = 'SELECT r.rowid FROM '.MAIN_DB_PREFIX.'c_exp_tax_range r'; if ($active) { $sql .= ' INNER JOIN '.MAIN_DB_PREFIX.'c_exp_tax_cat c ON (r.fk_c_exp_tax_cat = c.rowid)'; } - $sql .= ' WHERE r.fk_c_exp_tax_cat = '.$fk_c_exp_tax_cat; + $sql .= ' WHERE r.fk_c_exp_tax_cat = '.((int) $fk_c_exp_tax_cat); if ($active) { $sql .= ' AND r.active = 1 AND c.active = 1'; } $sql .= ' ORDER BY r.range_ik'; - dol_syslog(get_called_class().'::getRangesByCategory sql='.$sql, LOG_DEBUG); $resql = $db->query($sql); if ($resql) { $num = $db->num_rows($resql); diff --git a/htdocs/loan/calcmens.php b/htdocs/loan/calcmens.php index 5c6260dec84..19763c0a924 100644 --- a/htdocs/loan/calcmens.php +++ b/htdocs/loan/calcmens.php @@ -37,11 +37,11 @@ if (!defined('NOREQUIREAJAX')) { require '../main.inc.php'; require DOL_DOCUMENT_ROOT.'/core/lib/loan.lib.php'; -$mens = GETPOST('mens'); -$capital = GETPOST('capital'); -$rate = GETPOST('rate'); -$echance = GETPOST('echeance'); -$nbterm = GETPOST('nbterm'); +$mens = price2num(GETPOST('mens')); +$capital = price2num(GETPOST('capital')); +$rate = price2num(GETPOST('rate')); +$echance = GETPOST('echeance', 'int'); +$nbterm = GETPOST('nbterm', 'int'); top_httphead(); diff --git a/test/phpunit/SecurityTest.php b/test/phpunit/SecurityTest.php index 1d3b76cd4f2..616558f342a 100644 --- a/test/phpunit/SecurityTest.php +++ b/test/phpunit/SecurityTest.php @@ -316,6 +316,7 @@ class SecurityTest extends PHPUnit\Framework\TestCase $_GET["param4"]='../dir'; $_GET["param5"]="a_1-b"; $_POST["param6"]="">assertEquals('">', $result); + $result=GETPOST("param6b"); + print __METHOD__." result=".$result."\n"; + $this->assertEquals('abc', $result); + // With restricthtml we must remove html open/close tag and content but not htmlentities like n $result=GETPOST("param7", 'restricthtml');