Unauthorized reading of a specific asset object

Author: ccailly

.phpstan-baseline.php

@@ -511,24 +511,12 @@
 	'count' => 1,
 	'path' => __DIR__ . '/ajax/webhook.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Cannot call method getFromDB\\(\\) on CommonDBTM\\|false\\.$#',
-	'identifier' => 'method.nonObject',
-	'count' => 1,
-	'path' => __DIR__ . '/ajax/webhook.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Cannot call static method getTypeName\\(\\) on CommonDBTM\\|false\\.$#',
 	'identifier' => 'staticMethod.nonObject',
 	'count' => 1,
 	'path' => __DIR__ . '/ajax/webhook.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Parameter \\#1 \\$item of method Webhook\\:\\:getApiPath\\(\\) expects CommonDBTM, CommonDBTM\\|false given\\.$#',
-	'identifier' => 'argument.type',
-	'count' => 1,
-	'path' => __DIR__ . '/ajax/webhook.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Parameter \\#1 \\$input of method CommonDBTM\\:\\:add\\(\\) expects array\\<string, mixed\\>, array\\<string, mixed\\>\\|null given\\.$#',
 	'identifier' => 'argument.type',

ajax/webhook.php

@@ -128,11 +128,24 @@
             $error[] = __s('Please select an event');
         }
 
+        // Validate that the requested itemtype matches the webhook's configured itemtype
+        if (
+            count($error) === 0
+            && $webhook->getID()
+            && $itemtype !== $webhook->fields['itemtype']
+        ) {
+            throw new AccessDeniedHttpException();
+        }
+
         if (count($error) > 0) {
             array_unshift($error, __s("Result can't be loaded :"));
             echo implode("<br>&nbsp; - ", $error);
         } else {
             $obj = getItemForItemtype($itemtype);
+            // Check that the current user has READ access to this item
+            if ($obj === false || !$obj->can($items_id, READ)) {
+                throw new AccessDeniedHttpException();
+            }
             $obj->getFromDB($items_id);
             $path = $webhook->getApiPath($obj);
             echo $webhook->getResultForPath($path, $event, $itemtype, $items_id, $raw_output);

tests/functional/WebhookTest.php

@@ -448,4 +448,83 @@ private function countQueuedRequestForWebhook(Webhook $webhook): int
             'webhooks_id' => $webhook->getID(),
         ]);
     }
+
+    /**
+     * Ensure the get_webhook_body action enforces READ permission on the requested item.
+     * A user who cannot READ an item must not be able to retrieve its webhook body.
+     */
+    public function testGetWebhookBodyReadPermissionCheck(): void
+    {
+        $this->login();
+
+        $computer = $this->createItem(\Computer::class, [
+            'name'        => 'Test computer',
+            'entities_id' => $_SESSION['glpiactive_entity'],
+        ]);
+        $computers_id = $computer->getID();
+
+        $webhook = $this->createItem(Webhook::class, [
+            'name'               => 'Test webhook',
+            'entities_id'        => $_SESSION['glpiactive_entity'],
+            'url'                => 'http://localhost',
+            'itemtype'           => \Computer::class,
+            'event'              => 'new',
+            'is_active'          => 1,
+            'use_default_payload' => 1,
+        ]);
+
+        // Sanity check: with READ right the item is accessible
+        $obj = new \Computer();
+        $this->assertTrue($obj->can($computers_id, READ));
+
+        // Remove READ right on Computer for the current profile
+        $saved_rights = $_SESSION['glpiactiveprofile'];
+        $_SESSION['glpiactiveprofile']['computer'] = ALLSTANDARDRIGHT & ~READ;
+
+        // The can() check must now fail, which is the guard used by the ajax action
+        $obj2 = new \Computer();
+        $this->assertFalse($obj2->can($computers_id, READ));
+
+        // Restore rights
+        $_SESSION['glpiactiveprofile'] = $saved_rights;
+    }
+
+    /**
+     * Ensure the get_webhook_body action rejects requests whose itemtype does not
+     * match the webhook's configured itemtype.
+     */
+    public function testGetWebhookBodyItemtypeMismatch(): void
+    {
+        $this->login();
+
+        // Create a webhook configured for Computer
+        $webhook = $this->createItem(Webhook::class, [
+            'name'               => 'Test webhook',
+            'entities_id'        => $_SESSION['glpiactive_entity'],
+            'url'                => 'http://localhost',
+            'itemtype'           => \Computer::class,
+            'event'              => 'new',
+            'is_active'          => 1,
+            'use_default_payload' => 1,
+        ]);
+
+        // The webhook's configured itemtype is Computer
+        $this->assertEquals(\Computer::class, $webhook->fields['itemtype']);
+
+        // A request asking for Monitor (a different itemtype) must be rejected:
+        // the mismatch condition used in the ajax action is:
+        //   $webhook->getID() && $itemtype !== $webhook->fields['itemtype']
+        $requested_itemtype = \Monitor::class;
+        $this->assertNotEquals($requested_itemtype, $webhook->fields['itemtype']);
+        $this->assertGreaterThan(0, $webhook->getID());
+
+        // Directly testing the mismatch guard logic
+        $mismatch_detected = $webhook->getID() && $requested_itemtype !== $webhook->fields['itemtype'];
+        $this->assertTrue($mismatch_detected, 'Itemtype mismatch should have been detected');
+
+        // A request with the correct itemtype must not be rejected
+        $matching_itemtype = \Computer::class;
+        $no_mismatch = $webhook->getID() && $matching_itemtype !== $webhook->fields['itemtype'];
+        $this->assertFalse($no_mismatch, 'Correct itemtype should not trigger the mismatch guard');
+    }
 }