From 0ac30068ff0a7cc6523dd2134d673235ddb032b3 Mon Sep 17 00:00:00 2001
From: Rom1-B <8530352+Rom1-B@users.noreply.github.com>
Date: Tue, 28 Jul 2026 15:18:31 +0200
Subject: [PATCH] Fix: enforce report access checks on view and export
---
CHANGELOG.md | 4 ++++
front/preference.form.php | 4 ++++
front/profile.form.php | 2 +-
hook.php | 4 ++--
inc/common.class.php | 23 +++++++++++++++++++++++
inc/dashboard.class.php | 3 +++
inc/graphcsv.class.php | 21 ++++++++++++++++-----
inc/helpdesk.class.php | 4 ++--
inc/helpdeskplus.class.php | 6 +++---
psalm.xml | 3 +++
setup.php | 6 ++++--
11 files changed, 65 insertions(+), 15 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6d0e2052..4bba0e22 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## [UNRELEASED]
+### Fixed
+
+- Fix missing access and validation checks on several report and admin actions
+
## [1.10.0] - 2026-06-10
### Fixed
diff --git a/front/preference.form.php b/front/preference.form.php
index c411f54e..933d442d 100644
--- a/front/preference.form.php
+++ b/front/preference.form.php
@@ -37,6 +37,10 @@
$pref = new PluginMreportingPreference();
if (isset($_POST['update'])) {
+ if (!$pref->getFromDB($_POST['id']) || $pref->fields['users_id'] != Session::getLoginUserID()) {
+ Html::back();
+ }
+ $_POST['users_id'] = Session::getLoginUserID();
$pref->update($_POST);
Html::back();
} else {
diff --git a/front/profile.form.php b/front/profile.form.php
index c4eeaef3..fc5ed8ca 100644
--- a/front/profile.form.php
+++ b/front/profile.form.php
@@ -28,7 +28,7 @@
* -------------------------------------------------------------------------
*/
-Session::checkRight('profile', READ);
+Session::checkRight('profile', UPDATE);
/** @var DBmysql $DB */
global $DB;
diff --git a/hook.php b/hook.php
index 6274bd85..32e63953 100644
--- a/hook.php
+++ b/hook.php
@@ -361,8 +361,8 @@ function plugin_mreporting_giveItem($type, $ID, $data, $num)
}
}
}
- $out = "" .
- $data['raw']["ITEM_$num"] . ' (' . $title_func . ')';
+ $out = "" .
+ htmlspecialchars($data['raw']["ITEM_$num"], ENT_QUOTES) . ' (' . $title_func . ')';
}
return $out;
diff --git a/inc/common.class.php b/inc/common.class.php
index 7d778a99..eba36547 100644
--- a/inc/common.class.php
+++ b/inc/common.class.php
@@ -611,6 +611,24 @@ public static function initGraphParams(array $params): array
return $crit;
}
+ /**
+ * Reject the request unless the report is active and the current profile
+ * has read access to it (mirrors the check used to build the reports menu,
+ * but enforced here on the actual data-returning endpoints).
+ */
+ private static function checkReportAccess($f_name, $classname)
+ {
+ $config = new PluginMreportingConfig();
+ if (
+ !$config->getFromDBByFunctionAndClassname($f_name, $classname)
+ || !$config->fields['is_active']
+ || !isset($_SESSION['glpiactiveprofile']['id'])
+ || !PluginMreportingProfile::canViewReports($_SESSION['glpiactiveprofile']['id'], $config->fields['id'])
+ ) {
+ throw new NotFoundHttpException();
+ }
+ }
+
/**
* show Graph : Show graph
*
@@ -671,6 +689,7 @@ public function showGraph($opt, $export = false, $forceFormat = null)
if (!is_a($classname, PluginMreportingBaseclass::class, true)) {
return false;
}
+ self::checkReportAccess($opt['f_name'], $classname);
$obj = new $classname($config);
$datas = $obj->{$opt['f_name']}($config);
@@ -1115,6 +1134,9 @@ public function export($opt)
foreach ($report['functions'] as $func) {
foreach ($opt['check'] as $do => $to) {
if ($do == $func['function'] . $classname) {
+ if (!$func['is_active'] || !$func['right']) {
+ continue;
+ }
//dynamic instanciation of class passed by 'short_classname' GET parameter
$config = PluginMreportingConfig::initConfigParams($func['function'], $classname);
$class = 'PluginMreporting' . $func['short_classname'];
@@ -1206,6 +1228,7 @@ public function export($opt)
if (!is_a($classname, PluginMreportingBaseclass::class, true)) {
return false;
}
+ self::checkReportAccess($opt['f_name'], $classname);
$obj = new $classname($config);
//dynamic call of method passed by 'f_name' GET parameter with previously instancied class
diff --git a/inc/dashboard.class.php b/inc/dashboard.class.php
index 4aea427c..c0f3a0b1 100644
--- a/inc/dashboard.class.php
+++ b/inc/dashboard.class.php
@@ -271,6 +271,9 @@ public function getFormForColumn()
public static function removeReportFromDashboard($id)
{
$report = new PluginMreportingDashboard();
+ if (!$report->getFromDB($id) || $report->fields['users_id'] != Session::getLoginUserID()) {
+ return false;
+ }
return $report->delete(['id' => $id]);
}
diff --git a/inc/graphcsv.class.php b/inc/graphcsv.class.php
index 358fdc29..4351296d 100644
--- a/inc/graphcsv.class.php
+++ b/inc/graphcsv.class.php
@@ -42,6 +42,17 @@ public function initGraph($options)
}
}
+ /**
+ * Prefix a leading =, +, - or @ so spreadsheet software doesn't
+ * interpret the field as a formula.
+ */
+ private static function escapeCsvField($value)
+ {
+ $value = (string) $value;
+
+ return preg_match('/^[=+\-@]/', $value) ? "'" . $value : $value;
+ }
+
public function showHbar($params, $dashboard = false, $width = false)
{
/** @var array $CFG_GLPI */
@@ -91,7 +102,7 @@ public function showHbar($params, $dashboard = false, $width = false)
//titles
$out = $title . ' - ' . $desc . "\r\n";
foreach ($labels as $label) {
- $out .= $label . $CFG_GLPI['csv_delimiter'];
+ $out .= self::escapeCsvField($label) . $CFG_GLPI['csv_delimiter'];
}
$out = substr($out, 0, -1) . "\r\n";
@@ -101,7 +112,7 @@ public function showHbar($params, $dashboard = false, $width = false)
}
$out = substr($out, 0, -1) . "\r\n";
- echo htmlspecialchars($out);
+ echo $out;
}
public function showPie($params, $dashboard = false, $width = false)
@@ -158,7 +169,7 @@ public function showHgbar($params, $dashboard = false, $width = false)
foreach ($datas as $label2 => $cols) {
//title
- $out .= $label2 . "\r\n";
+ $out .= self::escapeCsvField($label2) . "\r\n";
//subtitle
$i = 0;
@@ -167,7 +178,7 @@ public function showHgbar($params, $dashboard = false, $width = false)
if (isset($labels2[$i])) {
$label = str_replace(',', '-', $labels2[$i]);
}
- $out .= $label . $CFG_GLPI['csv_delimiter'];
+ $out .= self::escapeCsvField($label) . $CFG_GLPI['csv_delimiter'];
$i++;
}
$out = substr($out, 0, -1) . "\r\n";
@@ -180,7 +191,7 @@ public function showHgbar($params, $dashboard = false, $width = false)
}
$out = substr($out, 0, -1) . "\r\n";
- echo htmlspecialchars($out);
+ echo $out;
}
public function showVstackbar($params, $dashboard = false, $width = false)
diff --git a/inc/helpdesk.class.php b/inc/helpdesk.class.php
index 45c16dcb..b64ba093 100644
--- a/inc/helpdesk.class.php
+++ b/inc/helpdesk.class.php
@@ -996,7 +996,7 @@ public function reportHbarTicketNumberByLocation($config = [])
$delay = PluginMreportingCommon::getCriteriaDate('glpi_tickets.date', $config['delay'], $config['randname']);
$datas = [];
- $limit = $_REQUEST['glpilist_limit'] ?? 20;
+ $limit = (int) ($_REQUEST['glpilist_limit'] ?? 20);
$query = [
"SELECT" => [
@@ -1034,7 +1034,7 @@ public function reportHbarTicketNumberByLocation($config = [])
],
'GROUPBY' => [Location::getTable() . '.name'],
'ORDER' => ['count DESC'],
- 'LIMIT' => '0, ' . $limit,
+ 'LIMIT' => $limit,
];
$query['WHERE']['AND'] = $delay;
diff --git a/inc/helpdeskplus.class.php b/inc/helpdeskplus.class.php
index 3aa26dd9..587bf34f 100644
--- a/inc/helpdeskplus.class.php
+++ b/inc/helpdeskplus.class.php
@@ -664,7 +664,7 @@ public function reportHbarTopcategory($config = [])
//Init delay value
$delay = PluginMreportingCommon::getCriteriaDate('glpi_tickets.date', $config['delay'], $config['randname']);
- $limit = $_SESSION['mreporting_values']['glpilist_limit'] ?? 20;
+ $limit = (int) ($_SESSION['mreporting_values']['glpilist_limit'] ?? 20);
$query = [
"SELECT" => [
@@ -718,7 +718,7 @@ public function reportHbarTopapplicant($config = [])
//Init delay value
$delay = PluginMreportingCommon::getCriteriaDate('glpi_tickets.date', $config['delay'], $config['randname']);
- $limit = $_SESSION['mreporting_values']['glpilist_limit'] ?? 20;
+ $limit = (int) ($_SESSION['mreporting_values']['glpilist_limit'] ?? 20);
$query = [
"SELECT" => [
@@ -1009,7 +1009,7 @@ public function reportHgbarRespectedSlasByTopCategory($config = [])
$category = isset($_POST['categories']) && $_POST['categories'] > 0 ? $_POST['categories'] : false;
- $category_limit = $_POST['glpilist_limit'] ?? 10;
+ $category_limit = (int) ($_POST['glpilist_limit'] ?? 10);
$_SESSION['glpilist_limit'] = $category_limit;
diff --git a/psalm.xml b/psalm.xml
index d5f56eac..7854012e 100644
--- a/psalm.xml
+++ b/psalm.xml
@@ -14,11 +14,14 @@
+
+
+
diff --git a/setup.php b/setup.php
index 96a7240c..7ce8783e 100644
--- a/setup.php
+++ b/setup.php
@@ -78,8 +78,10 @@ function plugin_init_mreporting()
if (Plugin::isPluginActive('mreporting')) {
// *Direct* access to rapport file (from e-mail) :
if (isset($_GET['redirect']) && str_contains($_GET['redirect'], 'plugin_mreporting')) {
- $filename = str_replace('plugin_mreporting_', '', $_GET['redirect']);
- Html::redirect($CFG_GLPI['root_doc'] . '/files/_plugins/mreporting/notifications/' . $filename);
+ $filename = basename(str_replace('plugin_mreporting_', '', $_GET['redirect']));
+ if (preg_match('/^[\w\-.]+$/', $filename)) {
+ Html::redirect($CFG_GLPI['root_doc'] . '/files/_plugins/mreporting/notifications/' . $filename);
+ }
}
//Load additionnal language files in needed