CVE-2023-28855: Improper Privilege Management (CWE-269)
value: Fields is a GLPI plugin that allows users to add custom fields on GLPI items forms. Prior to versions 1.13.1 and 1.20.4, lack of access control check allows any authenticated user to write data to any fields container, including those to which they have no configured access. Versions 1.13.1 and 1.20.4 contain a patch for this issue.
CWE description: The product does not properly assign, modify, track, or check privileges for an actor, creating an unintended sphere of control for that actor.
CVSS v3 base score: 6.5
Published: 2023-04-05T18:15Z
Repository: https://github.com/pluginsGLPI/fields
Language: PHP
Files changed: front/container.form.php, inc/container.class.php, inc/field.class.php, inc/profile.class.php
Commit date: 2023-04-05 08:54:17 +0200
Commit message: Merge pull request from GHSA-52vv-hm4x-8584


Diff: diff --git a/front/container.form.php b/front/container.form.php
index 706337a..1395b46 100644
--- a/front/container.form.php
+++ b/front/container.form.php
@@ -51,11 +51,14 @@ if (isset($_POST["add"])) {
 } else if (isset($_POST["update"])) {
     $container->check($_POST['id'], UPDATE);
     $container->update($_POST);
     Html::back();
 } else if (isset($_POST["update_fields_values"])) {
-    $container->updateFieldsValues($_REQUEST, $_REQUEST['itemtype'], false);
+    $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $_POST['plugin_fields_containers_id']);
+    if ($right > READ) {
+        $container->updateFieldsValues($_REQUEST, $_REQUEST['itemtype'], false);
+    }
     Html::back();
 } else {
     Html::header(
         __("Additional fields", "fields"),
         $_SERVER['PHP_SELF'],
diff --git a/inc/container.class.php b/inc/container.class.php
index f0c43bc..0e78de2 100644
--- a/inc/container.class.php
+++ b/inc/container.class.php
@@ -965,11 +965,10 @@ HTML;
             return false;
         }
 
         $itemtypes = [];
         $container = new self();
-        $profile   = new PluginFieldsProfile();
         $found     = $container->find($condition, 'label');
         foreach ($found as $item) {
             //entities restriction
             if (!in_array($item['entities_id'], $_SESSION['glpiactiveentities'])) {
                 if ($item['is_recursive'] == 1) {
@@ -984,16 +983,12 @@ HTML;
 
             if (Session::isCron() || !isset($_SESSION['glpiactiveprofile']['id'])) {
                 continue;
             }
             //profiles restriction
-            $found = $profile->find(['profiles_id' => $_SESSION['glpiactiveprofile']['id'],
-                'plugin_fields_containers_id' => $item['id'],
-                'right' => ['>=', READ]
-            ]);
-            $first_found = array_shift($found);
-            if (!$first_found || $first_found['right'] == null || $first_found['right'] == 0) {
+            $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $item['id']);
+            if ($right < READ) {
                 continue;
             }
 
             $jsonitemtypes = json_decode($item['itemtypes']);
             //show more info or not
@@ -1455,20 +1450,14 @@ HTML;
                 $id = $data['id'];
             }
         }
 
         //profiles restriction
-        if (isset($_SESSION['glpiactiveprofile']['id'])) {
-            $profile = new PluginFieldsProfile();
-            if (isset($id)) {
-                $found = $profile->find(['profiles_id' => $_SESSION['glpiactiveprofile']['id'],
-                    'plugin_fields_containers_id' => $id
-                ]);
-                $first_found = array_shift($found);
-                if ($first_found === null || $first_found['right'] == null || $first_found['right'] == 0) {
-                     return false;
-                }
+        if (isset($_SESSION['glpiactiveprofile']['id']) && $id > 0) {
+            $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $id);
+            if ($right < READ) {
+                return false;
             }
         }
 
         return $id;
     }
@@ -1549,13 +1538,20 @@ HTML;
                     return false;
                 }
             }
         }
 
-        //need to check if container is usable on this object entity
         $loc_c = new PluginFieldsContainer();
         $loc_c->getFromDB($c_id);
+
+        // check rights on $c_id
+        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $c_id);
+        if (($right > READ) === false) {
+            return;
+        }
+
+        // need to check if container is usable on this object entity
         $entities = [$loc_c->fields['entities_id']];
         if ($loc_c->fields['is_recursive']) {
             $entities = getSonsOf(getTableForItemType('Entity'), $loc_c->fields['entities_id']);
         }
 
diff --git a/inc/field.class.php b/inc/field.class.php
index ef15130..1148d87 100644
--- a/inc/field.class.php
+++ b/inc/field.class.php
@@ -720,17 +720,16 @@ class PluginFieldsField extends CommonDBChild
         $this->showFormButtons($options);
     }
 
     public static function showForTabContainer($c_id, $item)
     {
-        //profile restriction (for reading profile)
-        $profile = new PluginFieldsProfile();
-        $found = $profile->find(['profiles_id' => $_SESSION['glpiactiveprofile']['id'],
-            'plugin_fields_containers_id' => $c_id
-        ]);
-        $first_found = array_shift($found);
-        $canedit = ($first_found['right'] == CREATE);
+        //profile restriction
+        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $c_id);
+        if ($right < READ) {
+            return;
+        }
+        $canedit = $right > READ;
 
         //get fields for this container
         $field_obj = new self();
         $fields = $field_obj->find(['plugin_fields_containers_id' => $c_id, 'is_active' => 1], "ranking");
         echo "<form method='POST' action='" . Plugin::getWebDir('fields') . "/front/container.form.php'>";
@@ -792,12 +791,10 @@ class PluginFieldsField extends CommonDBChild
      *
      * @return void
      */
     public static function showForTab($params)
     {
-        Html::requireJs('tinymce');
-
         $item = $params['item'];
 
         $functions = array_column(debug_backtrace(), 'function');
 
         $subtype = isset($_SESSION['glpi_tabs'][strtolower($item::getType())]) ? $_SESSION['glpi_tabs'][strtolower($item::getType())] : "";
@@ -812,17 +809,25 @@ class PluginFieldsField extends CommonDBChild
         }
         // if we are in 'dom' or 'tab' type, no need for subtype ('domtab' specific)
         if ($type != 'domtab') {
             $subtype = "";
         }
+
         //find container (if not exist, do nothing)
         if (isset($_REQUEST['c_id'])) {
             $c_id = $_REQUEST['c_id'];
         } else if (!$c_id = PluginFieldsContainer::findContainer(get_Class($item), $type, $subtype)) {
             return false;
         }
 
+        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $c_id);
+        if ($right < READ) {
+            return;
+        }
+
+        Html::requireJs('tinymce');
+
         //need to check if container is usable on this object entity
         $loc_c = new PluginFieldsContainer();
         $loc_c->getFromDB($c_id);
         $entities = [$loc_c->fields['entities_id']];
         if ($loc_c->fields['is_recursive']) {
@@ -962,14 +967,22 @@ JAVASCRIPT
         if (empty($fields)) {
             return false;
         }
 
         //get object associated with this fields
-        $tmp = $fields;
-        $first_field = array_shift($tmp);
+        $first_field = reset($fields);
         $container_obj = new PluginFieldsContainer();
-        $container_obj->getFromDB($first_field['plugin_fields_containers_id']);
+        if (!$container_obj->getFromDB($first_field['plugin_fields_containers_id'])) {
+            return false;
+        }
+
+        // check if current profile can edit fields
+        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $container_obj->getID());
+        if ($right < READ) {
+            return;
+        }
+        $canedit = $right > READ;
 
         // Fill status overrides if needed
         if (in_array($item->getType(), PluginFieldsStatusOverride::getStatusItemtypes())) {
             $status_overrides = PluginFieldsStatusOverride::getOverridesForItem($container_obj->getID(), $item);
             foreach ($status_overrides as $status_override) {
@@ -992,33 +1005,13 @@ JAVASCRIPT
                 ]
             );
             $found_v = array_shift($found_values);
         }
 
-        // find profiles (to check if current profile can edit fields)
-        $fprofile = new PluginFieldsProfile();
-        $found_p = $fprofile->find(
-            [
-                'profiles_id' => $_SESSION['glpiactiveprofile']['id'],
-                'plugin_fields_containers_id' => $first_field['plugin_fields_containers_id'],
-            ]
-        );
-        $first_found_p = array_shift($found_p);
-
         // test status for "CommonITILObject" objects
-        if ($item instanceof CommonITILObject) {
-            $status = $item->fields['status'] ?? null;
-            if (
-                ($status !== null && in_array($status, $item->getClosedStatusArray()))
-                || $first_found_p['right'] != CREATE
-            ) {
-                $canedit = false;
-            }
-        } else {
-            if ($first_found_p['right'] != CREATE) {
-                $canedit = false;
-            }
+        if ($item instanceof CommonITILObject && in_array($item->fields['status'] ?? null, $item->getClosedStatusArray())) {
+            $canedit = false;
         }
 
         //show all fields
         foreach ($fields as &$field) {
             $field['itemtype'] = self::getType();
diff --git a/inc/profile.class.php b/inc/profile.class.php
index a66ff1f..d99decd 100644
--- a/inc/profile.class.php
+++ b/inc/profile.class.php
@@ -194,6 +194,24 @@ class PluginFieldsProfile extends CommonDBRelation
     {
         $fields_profile = new self();
         $fields_profile->deleteByCriteria(['profiles_id' => $profile->fields['id']]);
         return true;
     }
+
+    public static function getRightOnContainer(int $profile_id, int $container_id): int
+    {
+        global $DB;
+
+        $container_profile = $DB->request(
+            [
+                'SELECT'  => ['MAX' => 'right AS right'],
+                'FROM'    => self::getTable(),
+                'WHERE'   => [
+                    'profiles_id' => $profile_id,
+                    'plugin_fields_containers_id' => $container_id,
+                ],
+            ]
+        );
+
+        return (int)$container_profile->current()['right'];
+    }
 }

Vulnerable code:     $container->updateFieldsValues($_REQUEST, $_REQUEST['itemtype'], false);
        $profile   = new PluginFieldsProfile();
            $found = $profile->find(['profiles_id' => $_SESSION['glpiactiveprofile']['id'],
                'plugin_fields_containers_id' => $item['id'],
                'right' => ['>=', READ]
            ]);
            $first_found = array_shift($found);
            if (!$first_found || $first_found['right'] == null || $first_found['right'] == 0) {
        if (isset($_SESSION['glpiactiveprofile']['id'])) {
            $profile = new PluginFieldsProfile();
            if (isset($id)) {
                $found = $profile->find(['profiles_id' => $_SESSION['glpiactiveprofile']['id'],
                    'plugin_fields_containers_id' => $id
                ]);
                $first_found = array_shift($found);
                if ($first_found === null || $first_found['right'] == null || $first_found['right'] == 0) {
                     return false;
                }
        //need to check if container is usable on this object entity
        //profile restriction (for reading profile)
        $profile = new PluginFieldsProfile();
        $found = $profile->find(['profiles_id' => $_SESSION['glpiactiveprofile']['id'],
            'plugin_fields_containers_id' => $c_id
        ]);
        $first_found = array_shift($found);
        $canedit = ($first_found['right'] == CREATE);
        Html::requireJs('tinymce');

        $tmp = $fields;
        $first_field = array_shift($tmp);
        $container_obj->getFromDB($first_field['plugin_fields_containers_id']);
        // find profiles (to check if current profile can edit fields)
        $fprofile = new PluginFieldsProfile();
        $found_p = $fprofile->find(
            [
                'profiles_id' => $_SESSION['glpiactiveprofile']['id'],
                'plugin_fields_containers_id' => $first_field['plugin_fields_containers_id'],
            ]
        );
        $first_found_p = array_shift($found_p);

        if ($item instanceof CommonITILObject) {
            $status = $item->fields['status'] ?? null;
            if (
                ($status !== null && in_array($status, $item->getClosedStatusArray()))
                || $first_found_p['right'] != CREATE
            ) {
                $canedit = false;
            }
        } else {
            if ($first_found_p['right'] != CREATE) {
                $canedit = false;
            }
Fixed code:     $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $_POST['plugin_fields_containers_id']);
    if ($right > READ) {
        $container->updateFieldsValues($_REQUEST, $_REQUEST['itemtype'], false);
    }
            $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $item['id']);
            if ($right < READ) {
        if (isset($_SESSION['glpiactiveprofile']['id']) && $id > 0) {
            $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $id);
            if ($right < READ) {
                return false;

        // check rights on $c_id
        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $c_id);
        if (($right > READ) === false) {
            return;
        }

        // need to check if container is usable on this object entity
        //profile restriction
        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $c_id);
        if ($right < READ) {
            return;
        }
        $canedit = $right > READ;

        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $c_id);
        if ($right < READ) {
            return;
        }

        Html::requireJs('tinymce');

        $first_field = reset($fields);
        if (!$container_obj->getFromDB($first_field['plugin_fields_containers_id'])) {
            return false;
        }

        // check if current profile can edit fields
        $right = PluginFieldsProfile::getRightOnContainer($_SESSION['glpiactiveprofile']['id'], $container_obj->getID());
        if ($right < READ) {
            return;
        }
        $canedit = $right > READ;
        if ($item instanceof CommonITILObject && in_array($item->fields['status'] ?? null, $item->getClosedStatusArray())) {
            $canedit = false;

    public static function getRightOnContainer(int $profile_id, int $container_id): int
    {
        global $DB;

        $container_profile = $DB->request(
            [
                'SELECT'  => ['MAX' => 'right AS right'],
                'FROM'    => self::getTable(),
                'WHERE'   => [
                    'profiles_id' => $profile_id,
                    'plugin_fields_containers_id' => $container_id,
                ],
            ]
        );

        return (int)$container_profile->current()['right'];
    }