diff --git a/application/dashlet.class.inc.php b/application/dashlet.class.inc.php index 42eeca069e..0bc8979f47 100644 --- a/application/dashlet.class.inc.php +++ b/application/dashlet.class.inc.php @@ -255,7 +255,7 @@ abstract class Dashlet catch(OqlException $e) { $oPage->add('
'); - $oPage->p($e->GetUserFriendlyDescription()); + $oPage->p(utils::HtmlEntities($e->GetUserFriendlyDescription())); $oPage->add('
'); } catch(Exception $e) diff --git a/application/utils.inc.php b/application/utils.inc.php index f64cf306d0..3117f40b2d 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -283,6 +283,7 @@ class utils * * @since 2.5.2 2.6.0 new 'transaction_id' filter * @since 2.7.0 new 'element_identifier' filter + * @since 2.7.7, 3.0.2, 3.1.0 new 'url' N°4899 */ protected static function Sanitize_Internal($value, $sSanitizationFilter) { @@ -358,6 +359,11 @@ class utils $retValue = preg_replace('/[^a-zA-Z0-9_]/', '', $value); break; + // For URL + case 'url': + $retValue = filter_var($value, FILTER_SANITIZE_URL); + break; + default: case 'raw_data': $retValue = $value; diff --git a/core/action.class.inc.php b/core/action.class.inc.php index 5bb64979bd..16754df99d 100644 --- a/core/action.class.inc.php +++ b/core/action.class.inc.php @@ -179,7 +179,7 @@ class ActionEmail extends ActionNotification protected function FindRecipients($sRecipAttCode, $aArgs) { $sOQL = $this->Get($sRecipAttCode); - if (strlen($sOQL) == '') return ''; + if (strlen($sOQL) === 0) return ''; try { diff --git a/core/dbobject.class.php b/core/dbobject.class.php index c842fe6d2b..a068161159 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -1977,9 +1977,9 @@ abstract class DBObject implements iDisplay /** * check attributes together * - * @overwritable-hook You can extend this method in order to provide your own logic. - * - * @return bool + * @overwritable-hook You can extend this method in order to provide your own logic. + * + * @return true|string true if successful, the error description otherwise */ public function CheckConsistency() { diff --git a/core/displayablegraph.class.inc.php b/core/displayablegraph.class.inc.php index 66cd7e660d..a39a1a0fc1 100644 --- a/core/displayablegraph.class.inc.php +++ b/core/displayablegraph.class.inc.php @@ -1203,8 +1203,10 @@ class DisplayableGraph extends SimpleGraph * @param float $xMax Right coordinate of the bounding box to display the graph * @param float $yMin Top coordinate of the bounding box to display the graph * @param float $yMax Bottom coordinate of the bounding box to display the graph + * + * @since 2.7.7 3.0.2 3.1.0 N°4985 $sComments param is no longer optional */ - function RenderAsPDF(PDFPage $oPage, $sComments = '', $sContextKey, $xMin = -1, $xMax = -1, $yMin = -1, $yMax = -1) + function RenderAsPDF(PDFPage $oPage, $sComments, $sContextKey, $xMin = -1, $xMax = -1, $yMin = -1, $yMax = -1) { $aContextDefs = static::GetContextDefinitions($sContextKey, false); // No need to develop the parameters $oPdf = $oPage->get_tcpdf(); diff --git a/datamodels/2.x/itop-portal-base/portal/src/Brick/BrickCollection.php b/datamodels/2.x/itop-portal-base/portal/src/Brick/BrickCollection.php index 1fc733ff02..ed7f370d50 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Brick/BrickCollection.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Brick/BrickCollection.php @@ -162,12 +162,20 @@ class BrickCollection // - Home $this->aHomeOrdering = $this->aAllowedBricks; usort($this->aHomeOrdering, function (PortalBrick $a, PortalBrick $b) { - return $a->GetRankHome() > $b->GetRankHome(); + if ($a->GetRankHome() === $b->GetRankHome()) { + return 0; + } + + return $a->GetRankHome() > $b->GetRankHome() ? 1 : -1; }); // - Navigation menu $this->aNavigationMenuOrdering = $this->aAllowedBricks; usort($this->aNavigationMenuOrdering, function (PortalBrick $a, PortalBrick $b) { - return $a->GetRankNavigationMenu() > $b->GetRankNavigationMenu(); + if ($a->GetRankNavigationMenu() === $b->GetRankNavigationMenu()) { + return 0; + } + + return $a->GetRankNavigationMenu() > $b->GetRankNavigationMenu() ? 1 : -1; }); } diff --git a/datamodels/2.x/itop-portal-base/portal/src/Brick/ManageBrick.php b/datamodels/2.x/itop-portal-base/portal/src/Brick/ManageBrick.php index 9416eda0d0..8699f18492 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Brick/ManageBrick.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Brick/ManageBrick.php @@ -481,7 +481,11 @@ class ManageBrick extends PortalBrick if (!$this->IsGroupingByDistinctValues($sName)) { usort($this->aGrouping[$sName]['groups'], function ($a, $b) { - return $a['rank'] > $b['rank']; + if ($a['rank'] === $b['rank']) { + return 0; + } + + return $a['rank'] > $b['rank'] ? 1 : -1; }); } diff --git a/datamodels/2.x/itop-portal-base/portal/src/DependencyInjection/SilexCompatBootstrap/PortalXmlConfiguration/Lists.php b/datamodels/2.x/itop-portal-base/portal/src/DependencyInjection/SilexCompatBootstrap/PortalXmlConfiguration/Lists.php index bc12832ca7..e28deca520 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/DependencyInjection/SilexCompatBootstrap/PortalXmlConfiguration/Lists.php +++ b/datamodels/2.x/itop-portal-base/portal/src/DependencyInjection/SilexCompatBootstrap/PortalXmlConfiguration/Lists.php @@ -91,7 +91,10 @@ class Lists extends AbstractConfiguration } // - Sorting list items by rank usort($aListItems, function ($a, $b) { - return $a['rank'] > $b['rank']; + if ($a['rank'] == $b['rank']) { + return 0; + } + return $a['rank'] > $b['rank'] ? 1 : -1; }); $aClassLists[$sListId] = $aListItems; } diff --git a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php index 3b97ff5932..4f7041d50b 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php @@ -703,7 +703,7 @@ class ObjectFormManager extends FormManager /** @var Field $oField */ $oField = null; - if (is_callable(get_class($oAttDef).'::MakeFormField')) + if (is_callable([$oAttDef, 'MakeFormField'])) { $oField = $oAttDef->MakeFormField($this->oObject); } diff --git a/pages/ajax.render.php b/pages/ajax.render.php index fbe25a1525..dead514019 100644 --- a/pages/ajax.render.php +++ b/pages/ajax.render.php @@ -1183,7 +1183,7 @@ try $aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data'); $sDashboardFile = utils::ReadParam('file', '', false, 'raw_data'); - $sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data'); + $sReloadURL = utils::ReadParam('reload_url', '', false, 'url'); $oDashboard = RuntimeDashboard::GetDashboard($sDashboardFile, $sDashboardId); $aResult = array('error' => ''); if (!is_null($oDashboard)) @@ -1202,7 +1202,7 @@ try $sDashboardId = utils::ReadParam('dashboard_id', '', false, 'raw_data'); $aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data'); $sDashboardFile = utils::ReadParam('file', '', false, 'raw_data'); - $sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data'); + $sReloadURL = utils::ReadParam('reload_url', '', false, 'url'); $oDashboard = RuntimeDashboard::GetDashboard($sDashboardFile, $sDashboardId); $aResult = array('error' => ''); if (!is_null($oDashboard)) @@ -1219,7 +1219,7 @@ try case 'save_dashboard': $sDashboardId = utils::ReadParam('dashboard_id', '', false, 'context_param'); $aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data'); - $sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data'); + $sReloadURL = utils::ReadParam('reload_url', '', false, 'url'); $sJSExtraParams = json_encode($aExtraParams); $aParams = array(); $aParams['layout_class'] = utils::ReadParam('layout_class', ''); @@ -1252,7 +1252,7 @@ JS case 'revert_dashboard': $sDashboardId = utils::ReadParam('dashboard_id', '', false, 'raw_data'); - $sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data'); + $sReloadURL = utils::ReadParam('reload_url', '', false, 'url'); appUserPreferences::UnsetPref('display_original_dashboard_'.$sDashboardId); $oDashboard = new RuntimeDashboard($sDashboardId); $oDashboard->Revert(); @@ -1282,7 +1282,7 @@ EOF $aParams['cells'] = utils::ReadParam('cells', array(), false, 'raw_data'); $aParams['auto_reload'] = utils::ReadParam('auto_reload', false); $aParams['auto_reload_sec'] = utils::ReadParam('auto_reload_sec', 300); - $sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data'); + $sReloadURL = utils::ReadParam('reload_url', '', false, 'url'); $oKPI = new ExecutionKPI(); $oDashboard = new RuntimeDashboard($sDashboardId); $oDashboard->FromParams($aParams); @@ -1296,7 +1296,7 @@ EOF $aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data'); $aExtraParams['dashboard_div_id'] = utils::Sanitize($sId, '', 'element_identifier'); $sDashboardFile = utils::ReadParam('file', '', false, 'string'); - $sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data'); + $sReloadURL = utils::ReadParam('reload_url', '', false, 'url'); $oKPI = new ExecutionKPI(); $oDashboard = RuntimeDashboard::GetDashboard($sDashboardFile, $sId); if (!is_null($oDashboard)) diff --git a/pages/csvimport.php b/pages/csvimport.php index 9f4b566a23..a8ce680f63 100644 --- a/pages/csvimport.php +++ b/pages/csvimport.php @@ -197,6 +197,11 @@ try { throw new CoreException(Dict::S('UI:ActionNotAllowed')); } + // CSRF transaction id verification + if(!$bSimulate && !utils::IsTransactionValid(utils::ReadPostedParam('transaction_id', '', 'raw_data'))){ + throw new CoreException(Dict::S('UI:Error:InvalidToken')); + } + $aResult = array(); $sCSVData = utils::ReadParam('csvdata', '', false, 'raw_data'); $sCSVDataTruncated = utils::ReadParam('csvdata_truncated', '', false, 'raw_data'); @@ -487,11 +492,12 @@ try { $sHtml .= "$sMessage"; $sHtml .= ''; } - + $iUnchanged = count($aRes) - $iErrors - $iModified - $iCreated; $sHtml .= ''; $oPage->add('
'); $oPage->add('
'); + $oPage->add(''); $oPage->add(''); $oPage->add(''); $oPage->add(''); diff --git a/test/application/UtilsTest.php b/test/application/UtilsTest.php index f2d671fad0..1c800d57f0 100644 --- a/test/application/UtilsTest.php +++ b/test/application/UtilsTest.php @@ -22,7 +22,7 @@ /** * @covers utils */ -class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase +class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase { public function testEndsWith() { @@ -441,4 +441,50 @@ class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase '2G' => ['2G', 2 * 1024 * 1024 * 1024], ]; } + + /** + * Test sanitizer. + * + * @param $type string type of sanitizer + * @param $valueToSanitize ? value to sanitize + * @param $expectedResult ? expected result + * + * @return void + * + * @dataProvider sanitizerDataProvider + */ + public function testSanitizer($type, $valueToSanitize, $expectedResult) + { + $this->assertEquals($expectedResult, utils::Sanitize($valueToSanitize, null, $type), 'url sanitize failed'); + } + + /** + * DataProvider for testSanitizer + * + * @return array + */ + public function sanitizerDataProvider() + { + return [ + 'good integer' => ['integer', '2565', '2565'], + 'bad integer' => ['integer', 'a2656', '2656'], + 'good class' => ['class', 'UserRequest', 'UserRequest'], + 'bad class' => ['class', 'MyUserRequest',null], + 'good string' => ['string', 'Is Peter smart and funny?', 'Is Peter smart and funny?'], + 'bad string' => ['string', 'Is Peter & funny?', 'Is Peter <smart> & funny?'], + 'good transaction_id' => ['transaction_id', '8965.-dd', '8965.-dd'], + 'bad transaction_id' => ['transaction_id', '8965.-dd+', null], + 'good parameter' => ['parameter', 'JU8965-dd=_', 'JU8965-dd=_'], + 'bad parameter' => ['parameter', '8965.-dd+', null], + 'good field_name' => ['field_name', 'Name->bUzz38', 'Name->bUzz38'], + 'bad field_name' => ['field_name', 'name-buzz', null], + 'good context_param' => ['context_param', '%dssD25_=%:+-', '%dssD25_=%:+-'], + 'bad context_param' => ['context_param', '%dssD,25_=%:+-', null], + 'good element_identifier' => ['element_identifier', 'AD05nb', 'AD05nb'], + 'bad element_identifier' => ['element_identifier', 'AD05nb+', 'AD05nb'], + 'good url' => ['url', 'https://www.w3schools.com', 'https://www.w3schools.com'], + 'bad url' => ['url', 'https://www.w3schoo��ls.co�m', 'https://www.w3schools.com'], + 'raw_data' => ['raw_data', '\s😃😃😃', '\s😃😃😃'], + ]; + } } diff --git a/test/phpunit.xml.dist b/test/phpunit.xml.dist index 47ecd5e43b..1dcf5848d7 100644 --- a/test/phpunit.xml.dist +++ b/test/phpunit.xml.dist @@ -44,10 +44,6 @@ itop-tickets - - setup diff --git a/test/phpunit_nightly.xml.dist b/test/phpunit_nightly.xml.dist new file mode 100644 index 0000000000..684f4b6ba7 --- /dev/null +++ b/test/phpunit_nightly.xml.dist @@ -0,0 +1,42 @@ + + + + + + + + + + + + + OQL + + + + + + + ../core/apc-emulation.php + ../core/ormlinkset.class.inc.php + ../datamodels/2.x/itop-tickets/main.itop-tickets.php + + + +