From 29677394554a04248893947d70cb2090bb84765f Mon Sep 17 00:00:00 2001 From: Rom1-B <8530352+Rom1-B@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:02:20 +0200 Subject: [PATCH 1/2] Fix: validate item type and rights in lifecycle actions --- CHANGELOG.md | 6 ++++++ ajax/locations.php | 4 ++-- front/action.php | 9 ++++++++- inc/model.class.php | 2 +- inc/replace.class.php | 26 ++++++++++++++++++++++---- inc/uninstall.class.php | 9 +++++++-- 6 files changed, 46 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 859cc1e..92a7402 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [Unreleased] + +### Fixed + +- Fix uninstall/replace actions processing items without checking item type or user rights on the item + ## [2.10.4] - 2026-08-04 ### Fixed diff --git a/ajax/locations.php b/ajax/locations.php index db581e2..033cf69 100644 --- a/ajax/locations.php +++ b/ajax/locations.php @@ -31,14 +31,14 @@ header("Content-Type: text/html; charset=UTF-8"); Html::header_nocache(); -Session::checkRightsOr('uninstall:profile', [READ, PluginUninstallProfile::RIGHT_REPLACE]); +Session::checkRightsOr(PluginUninstallUninstall::$rightname, [READ, PluginUninstallProfile::RIGHT_REPLACE]); if ( Session::haveRight(PluginUninstallUninstall::$rightname, READ) && $_POST['templates_id'] ) { $location = PluginUninstallPreference::getLocationByUserByEntity( - $_POST["users_id"], + Session::getLoginUserID(), $_POST["templates_id"], $_POST["entity"], ); diff --git a/front/action.php b/front/action.php index f32ae07..3adaa9c 100644 --- a/front/action.php +++ b/front/action.php @@ -30,7 +30,7 @@ Html::header(__s('Transfer'), $_SERVER['PHP_SELF'], "admin", "transfer"); -Session::checkRightsOr('uninstall:profile', [READ, PluginUninstallProfile::RIGHT_REPLACE]); +Session::checkRightsOr(PluginUninstallUninstall::$rightname, [READ, PluginUninstallProfile::RIGHT_REPLACE]); if ( !isset($_REQUEST["device_type"]) @@ -40,6 +40,12 @@ Html::back(); } +/** @var array $UNINSTALL_TYPES */ +global $UNINSTALL_TYPES; +if (!in_array($_REQUEST["device_type"], $UNINSTALL_TYPES, true)) { + Html::back(); +} + if (isset($_REQUEST["locations_id"])) { $location = $_REQUEST["locations_id"]; } else { @@ -51,6 +57,7 @@ } if (isset($_REQUEST["replace"])) { + Session::checkRight(PluginUninstallUninstall::$rightname, PluginUninstallProfile::RIGHT_REPLACE); PluginUninstallReplace::replace( $_REQUEST["device_type"], $_REQUEST["model_id"], diff --git a/inc/model.class.php b/inc/model.class.php index e04c9b6..33e7da2 100644 --- a/inc/model.class.php +++ b/inc/model.class.php @@ -279,7 +279,7 @@ public function showForm($ID, $options = []) echo ""; echo "" . __s('Comments') . ""; echo ""; - echo ""; + echo ""; echo ""; echo "" . __s('New status of the computer', 'uninstall') . ""; diff --git a/inc/replace.class.php b/inc/replace.class.php index 0c495e3..bef2803 100644 --- a/inc/replace.class.php +++ b/inc/replace.class.php @@ -114,10 +114,28 @@ public static function replace($type, $model_id, $tab_ids, $location) } $olditem = new $type(); - $olditem->getFromDB($olditem_id); + if (!$olditem->getFromDB($olditem_id) || !$olditem->can($olditem_id, UPDATE)) { + continue; + } $newitem = new $type(); - $newitem->getFromDB($newitem_id); + if (!$newitem->getFromDB($newitem_id) || !$newitem->can($newitem_id, UPDATE)) { + continue; + } + + if ( + $model->fields['replace_method'] == self::METHOD_PURGE + && !$olditem->can($olditem_id, PURGE) + ) { + continue; + } + + if ( + $model->fields['replace_method'] == self::METHOD_DELETE_AND_COMMENT + && !$olditem->can($olditem_id, DELETE) + ) { + continue; + } //Hook to perform actions before item is being replaced $olditem->fields['_newid'] = $newitem_id; @@ -839,11 +857,11 @@ public static function showReplacementForm($type, $model_id, $tab_ids, $location echo "" . $commonitem->getName() . ""; if (Search::getOptionNumber($type, 'otherserial')) { - echo "" . $commonitem->fields['otherserial'] . ""; + echo "" . htmlentities((string) $commonitem->fields['otherserial']) . ""; } if (Search::getOptionNumber($type, 'serial')) { - echo "" . $commonitem->fields['serial'] . ""; + echo "" . htmlentities((string) $commonitem->fields['serial']) . ""; } echo ""; diff --git a/inc/uninstall.class.php b/inc/uninstall.class.php index 6ff8ee9..2757eb0 100644 --- a/inc/uninstall.class.php +++ b/inc/uninstall.class.php @@ -117,10 +117,12 @@ public static function processMassiveActionsForOneItemtype(MassiveAction $ma, Co if ($ma->getAction() === "uninstall") { $itemtype = $ma->getItemtype(false); foreach ($ids as $id) { - if ($item->getFromDB($id)) { + if ($item->getFromDB($id) && $item->can($id, UPDATE)) { //Session::addMessageAfterRedirect(sprintf(__s('Form duplicated: %s', 'formcreator'), $item->getName())); $_SESSION['glpi_uninstalllist'][$itemtype][$id] = $id; $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_OK); + } else { + $ma->itemDone($item->getType(), $id, MassiveAction::ACTION_NORIGHT); } } @@ -373,7 +375,9 @@ public static function uninstall($type, $model_id, $tab_ids, $location) $count++; if (class_exists($type) && is_a($type, CommonDBTM::class, true)) { $item = new $type(); - $item->getFromDB($id); + if (!$item->getFromDB($id) || !$item->can($id, UPDATE)) { + continue; + } self::doOneUninstall($model, $transfer, $item, [ 'type' => $type, @@ -476,6 +480,7 @@ public static function deleteComputerInOCS($ocs_id, $ocs_server_id) global $DB; if (class_exists('PluginOcsinventoryngOcsServer')) { $DBocs = PluginOcsinventoryngOcsServer::getDBocs($ocs_server_id)->getDB(); + $ocs_id = (int) $ocs_id; //First try to remove all the network ports $query = "DELETE From 67a316bec9c0357f00f4945f98b166ffa21019b7 Mon Sep 17 00:00:00 2001 From: Rom1-B <8530352+Rom1-B@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:52:17 +0200 Subject: [PATCH 2/2] stan --- front/action.php | 14 ++++++++++++-- inc/replace.class.php | 22 ++++++++++++++++++---- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/front/action.php b/front/action.php index 3adaa9c..769bf26 100644 --- a/front/action.php +++ b/front/action.php @@ -58,7 +58,7 @@ if (isset($_REQUEST["replace"])) { Session::checkRight(PluginUninstallUninstall::$rightname, PluginUninstallProfile::RIGHT_REPLACE); - PluginUninstallReplace::replace( + $skipped = PluginUninstallReplace::replace( $_REQUEST["device_type"], $_REQUEST["model_id"], $_REQUEST['newItems'], @@ -66,7 +66,15 @@ ); unset($_SESSION['glpi_uninstalllist']); - Session::addMessageAfterRedirect(__s('Replacement successful', 'uninstall')); + if ($skipped > 0) { + Session::addMessageAfterRedirect( + sprintf(__s('Replacement done with %d item(s) skipped because of insufficient rights', 'uninstall'), $skipped), + true, + WARNING, + ); + } else { + Session::addMessageAfterRedirect(__s('Replacement successful', 'uninstall')); + } Html::footer(); @@ -79,6 +87,7 @@ //Case of a uninstallation initiated from the object form if (isset($_REQUEST["uninstall"])) { + Session::checkRight(PluginUninstallUninstall::$rightname, UPDATE); //Uninstall only if a model is selected if ($model->fields['types_id'] == PluginUninstallModel::TYPE_MODEL_UNINSTALL) { //Massive uninstallation @@ -104,6 +113,7 @@ Html::footer(); } } elseif ($model->fields['types_id'] == PluginUninstallModel::TYPE_MODEL_UNINSTALL) { + Session::checkRight(PluginUninstallUninstall::$rightname, UPDATE); //Massive uninstallation if (isset($_SESSION['glpi_uninstalllist'])) { PluginUninstallUninstall::uninstall( diff --git a/inc/replace.class.php b/inc/replace.class.php index bef2803..15839d1 100644 --- a/inc/replace.class.php +++ b/inc/replace.class.php @@ -85,7 +85,7 @@ public static function getTypeName($nb = 0) * @param $tab_ids * @param $location **/ - public static function replace($type, $model_id, $tab_ids, $location) + public static function replace($type, $model_id, $tab_ids, $location): int { /** * @var array $CFG_GLPI @@ -103,23 +103,27 @@ public static function replace($type, $model_id, $tab_ids, $location) echo "
"; echo ""; echo ""; + if ($skipped > 0) { + echo ""; + } + echo "
" . __s('Replacement', 'uninstall') . "
"; - $count = 0; - $tot = count($tab_ids); + $count = 0; + $skipped = 0; + $tot = count($tab_ids); foreach ($tab_ids as $olditem_id => $newitem_id) { $count++; if (!class_exists($type) || !is_a($type, CommonDBTM::class, true)) { + $skipped++; continue; } $olditem = new $type(); if (!$olditem->getFromDB($olditem_id) || !$olditem->can($olditem_id, UPDATE)) { + $skipped++; continue; } $newitem = new $type(); if (!$newitem->getFromDB($newitem_id) || !$newitem->can($newitem_id, UPDATE)) { + $skipped++; continue; } @@ -127,6 +131,7 @@ public static function replace($type, $model_id, $tab_ids, $location) $model->fields['replace_method'] == self::METHOD_PURGE && !$olditem->can($olditem_id, PURGE) ) { + $skipped++; continue; } @@ -134,6 +139,7 @@ public static function replace($type, $model_id, $tab_ids, $location) $model->fields['replace_method'] == self::METHOD_DELETE_AND_COMMENT && !$olditem->can($olditem_id, DELETE) ) { + $skipped++; continue; } @@ -594,7 +600,13 @@ public static function replace($type, $model_id, $tab_ids, $location) Html::getProgressBar($percent); } - echo "
" . sprintf( + __s('%d item(s) skipped because of insufficient rights', 'uninstall'), + $skipped, + ) . "
"; if ($model->fields['types_id'] == PluginUninstallModel::TYPE_MODEL_REPLACEMENT_UNINSTALL) { @@ -610,6 +622,8 @@ public static function replace($type, $model_id, $tab_ids, $location) $location, ); } + + return $skipped; }