CVE-2024-29889: Insufficient Information (NVD-CWE-noinfo)
value: GLPI is a Free Asset and IT Management Software package. Prior to 10.0.15, an authenticated user can exploit a SQL injection vulnerability in the saved searches feature to alter another user account data take control of it. This vulnerability is fixed in 10.0.15.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-07T14:15Z
Repository: https://github.com/glpi-project/glpi
Language: PHP
Files changed: ajax/pin_savedsearches.php, src/User.php, tests/functional/User.php
Commit date: 2024-04-24 09:24:08 +0200
Commit message: Merge pull request from GHSA-8xvf-v6vv-r75g


Diff: diff --git a/ajax/pin_savedsearches.php b/ajax/pin_savedsearches.php
index 0eaf0ae530bb..2abd6095696f 100644
--- a/ajax/pin_savedsearches.php
+++ b/ajax/pin_savedsearches.php
@@ -38,24 +38,18 @@ include('../inc/includes.php');
 header('Content-Type: application/json; charset=UTF-8');
 Html::header_nocache();
 
 Session::checkLoginUser();
 
-if (!is_string($_POST['itemtype']) || getItemForItemtype($_POST['itemtype']) === false) {
-    echo json_encode(['success' => false]);
-    exit();
-}
-
-$all_pinned = importArrayFromDB($_SESSION['glpisavedsearches_pinned']);
-$already_pinned = $all_pinned[$_POST['itemtype']] ?? 0;
-$all_pinned[$_POST['itemtype']] = $already_pinned ? 0 : 1;
-$_SESSION['glpisavedsearches_pinned'] = exportArrayToDB($all_pinned);
+$success = false;
 
 $user = new User();
-$success = $user->update(
-    [
-        'id'                   => Session::getLoginUserID(),
-        'savedsearches_pinned' => $_SESSION['glpisavedsearches_pinned'],
-    ]
-);
+if (
+    array_key_exists('itemtype', $_POST)
+    && is_string($_POST['itemtype'])
+    && $user->getFromDB(Session::getLoginUserID())
+    && $user->toggleSavedSearchPin($_POST['itemtype'])
+) {
+    $success = true;
+}
 
 echo json_encode(['success' => $success]);
diff --git a/src/User.php b/src/User.php
index bf4eaac432ab..0451c155c6dd 100644
--- a/src/User.php
+++ b/src/User.php
@@ -6874,6 +6874,30 @@ HTML;
 
     public function willProcessRuleRight(): void
     {
         $this->must_process_ruleright = true;
     }
+
+    /**
+     * Toggle pin of given itemtype saved search.
+     *
+     * @param string $itemtype
+     *
+     * @return bool
+     */
+    public function toggleSavedSearchPin(string $itemtype): bool
+    {
+        if (getItemForItemtype($itemtype) === false) {
+            return false;
+        }
+
+        $all_pinned     = importArrayFromDB($this->fields['savedsearches_pinned']);
+        $already_pinned = $all_pinned[$itemtype] ?? 0;
+
+        $all_pinned[$itemtype] = $already_pinned ? 0 : 1;
+
+        return $this->update(Sanitizer::sanitize([
+            'id'                   => $this->fields['id'],
+            'savedsearches_pinned' => exportArrayToDB($all_pinned),
+        ]));
+    }
 }
diff --git a/tests/functional/User.php b/tests/functional/User.php
index 539c8718998a..ca5fcde71f8e 100644
--- a/tests/functional/User.php
+++ b/tests/functional/User.php
@@ -1402,6 +1402,72 @@ class User extends \DbTestCase
         $this->boolean($retrievedUser->getFromDBbyDn($dn))->isTrue();
         $this->boolean($retrievedUser->getFromDBbyDn(Sanitizer::sanitize($dn)))->isTrue(); // works also with sanitized value
         $this->boolean($retrievedUser->isNewItem())->isFalse();
         $this->string($retrievedUser->fields['user_dn'])->isEmpty();
     }
+
+    protected function toggleSavedSearchPinProvider(): iterable
+    {
+        foreach (['', '[]', '{}'] as $initial_db_value) {
+            // initial empty data
+            yield [
+                'initial_db_value' => $initial_db_value,
+                'itemtype'         => 'Computer',
+                'success'          => true,
+                'result_db_value'  => '{"Computer":1}',
+            ];
+        }
+
+        // toggle to 1
+        yield [
+            'initial_db_value' => '{"Computer":0,"Monitor":1}',
+            'itemtype'         => 'Computer',
+            'success'          => true,
+            'result_db_value'  => '{"Computer":1,"Monitor":1}',
+        ];
+
+        // toggle to 0
+        yield [
+            'initial_db_value' => '{"Computer":1,"Monitor":1}',
+            'itemtype'         => 'Monitor',
+            'success'          => true,
+            'result_db_value'  => '{"Computer":1,"Monitor":0}',
+        ];
+
+        // namespaced itemtype
+        yield [
+            'initial_db_value' => '{"Computer":1,"Monitor":0}',
+            'itemtype'         => 'Glpi\\Socket',
+            'success'          => true,
+            'result_db_value'  => '{"Computer":1,"Monitor":0,"Glpi\\\\Socket":1}',
+        ];
+
+        // invalid itemtype
+        yield [
+            'initial_db_value' => '{"Computer":1,"Monitor":1}',
+            'itemtype'         => 'This is not a valid itemtype',
+            'success'          => false,
+            'result_db_value'  => '{"Computer":1,"Monitor":1}',
+        ];
+    }
+
+    /**
+     * @dataProvider toggleSavedSearchPinProvider
+     */
+    public function testToggleSavedSearchPin(string $initial_db_value, string $itemtype, bool $success, string $result_db_value): void
+    {
+        $user = $this->createItem(
+            \User::class,
+            [
+                'name'                  => __FUNCTION__ . (string) mt_rand(),
+                'savedsearches_pinned'  => $initial_db_value,
+            ]
+        );
+
+        $this->boolean($user->toggleSavedSearchPin($itemtype))->isEqualTo($success);
+        $this->boolean($user->getFromDb($user->getID()))->isTrue();
+        $this->string($user->fields['savedsearches_pinned'])->isEqualTo($result_db_value);
+
+        // result value in DB is always a valid JSON string
+        $this->array(importArrayFromDB($user->fields['savedsearches_pinned']))->isEqualTo(json_decode($result_db_value, true));
+    }
 }

Vulnerable code: if (!is_string($_POST['itemtype']) || getItemForItemtype($_POST['itemtype']) === false) {
    echo json_encode(['success' => false]);
    exit();
}

$all_pinned = importArrayFromDB($_SESSION['glpisavedsearches_pinned']);
$already_pinned = $all_pinned[$_POST['itemtype']] ?? 0;
$all_pinned[$_POST['itemtype']] = $already_pinned ? 0 : 1;
$_SESSION['glpisavedsearches_pinned'] = exportArrayToDB($all_pinned);
$success = $user->update(
    [
        'id'                   => Session::getLoginUserID(),
        'savedsearches_pinned' => $_SESSION['glpisavedsearches_pinned'],
    ]
);
Fixed code: $success = false;
if (
    array_key_exists('itemtype', $_POST)
    && is_string($_POST['itemtype'])
    && $user->getFromDB(Session::getLoginUserID())
    && $user->toggleSavedSearchPin($_POST['itemtype'])
) {
    $success = true;
}

    /**
     * Toggle pin of given itemtype saved search.
     *
     * @param string $itemtype
     *
     * @return bool
     */
    public function toggleSavedSearchPin(string $itemtype): bool
    {
        if (getItemForItemtype($itemtype) === false) {
            return false;
        }

        $all_pinned     = importArrayFromDB($this->fields['savedsearches_pinned']);
        $already_pinned = $all_pinned[$itemtype] ?? 0;

        $all_pinned[$itemtype] = $already_pinned ? 0 : 1;

        return $this->update(Sanitizer::sanitize([
            'id'                   => $this->fields['id'],
            'savedsearches_pinned' => exportArrayToDB($all_pinned),
        ]));
    }

    protected function toggleSavedSearchPinProvider(): iterable
    {
        foreach (['', '[]', '{}'] as $initial_db_value) {
            // initial empty data
            yield [
                'initial_db_value' => $initial_db_value,
                'itemtype'         => 'Computer',
                'success'          => true,
                'result_db_value'  => '{"Computer":1}',
            ];
        }

        // toggle to 1
        yield [
            'initial_db_value' => '{"Computer":0,"Monitor":1}',
            'itemtype'         => 'Computer',
            'success'          => true,
            'result_db_value'  => '{"Computer":1,"Monitor":1}',
        ];

        // toggle to 0
        yield [
            'initial_db_value' => '{"Computer":1,"Monitor":1}',
            'itemtype'         => 'Monitor',
            'success'          => true,
            'result_db_value'  => '{"Computer":1,"Monitor":0}',
        ];

        // namespaced itemtype
        yield [
            'initial_db_value' => '{"Computer":1,"Monitor":0}',
            'itemtype'         => 'Glpi\\Socket',
            'success'          => true,
            'result_db_value'  => '{"Computer":1,"Monitor":0,"Glpi\\\\Socket":1}',
        ];

        // invalid itemtype
        yield [
            'initial_db_value' => '{"Computer":1,"Monitor":1}',
            'itemtype'         => 'This is not a valid itemtype',
            'success'          => false,
            'result_db_value'  => '{"Computer":1,"Monitor":1}',
        ];
    }

    /**
     * @dataProvider toggleSavedSearchPinProvider
     */
    public function testToggleSavedSearchPin(string $initial_db_value, string $itemtype, bool $success, string $result_db_value): void
    {
        $user = $this->createItem(
            \User::class,
            [
                'name'                  => __FUNCTION__ . (string) mt_rand(),
                'savedsearches_pinned'  => $initial_db_value,
            ]
        );

        $this->boolean($user->toggleSavedSearchPin($itemtype))->isEqualTo($success);
        $this->boolean($user->getFromDb($user->getID()))->isTrue();
        $this->string($user->fields['savedsearches_pinned'])->isEqualTo($result_db_value);

        // result value in DB is always a valid JSON string
        $this->array(importArrayFromDB($user->fields['savedsearches_pinned']))->isEqualTo(json_decode($result_db_value, true));
    }