Merge pull request from GHSA-92x4-q9w5-837w

* Check for valid itemtype before creating objects * Restrict HTTP methods allowed by Guzzle client --------- Co-authored-by: Cédric Anne <cedric.anne@gmail.com>
glpi-project · Mar 13, 2024 · 3b6bc1b · 3b6bc1b
1 parent da62dcc
commit 3b6bc1b
Show file tree

Hide file tree

Showing 18 changed files with 57 additions and 28 deletions.
diff --git a/ajax/comments.php b/ajax/comments.php
@@ -120,7 +120,7 @@
                 }
 
                 if (isset($_POST['with_dc_position'])) {
-                    $item = new $_POST['itemtype']();
+                    $item = getItemForItemtype($_POST['itemtype']);
                     echo "<script type='text/javascript' >\n";
 
                    //if item have a DC position (reload url to it's rack)

diff --git a/ajax/dropdownLocation.php b/ajax/dropdownLocation.php
@@ -45,7 +45,7 @@
     throw new \RuntimeException('Required argument missing or incorrect!');
 }
 
-$item = new $_REQUEST['itemtype']();
+$item = getItemForItemtype($_REQUEST['itemtype']);
 $item->getFromDB((int) $_REQUEST['items_id']);
 
 $locations_id = $item->fields['locations_id'] ?? 0;

diff --git a/ajax/kanban.php b/ajax/kanban.php
@@ -170,7 +170,7 @@
 } else if (($_POST['action'] ?? null) === 'move_item') {
     $checkParams(['card', 'column', 'position', 'kanban']);
     /** @var Kanban|CommonDBTM $kanban */
-    $kanban = new $_POST['kanban']['itemtype']();
+    $kanban = getItemForItemtype($_POST['kanban']['itemtype']);
     $can_move = $kanban->canOrderKanbanCard($_POST['kanban']['items_id']);
     if ($can_move) {
         Item_Kanban::moveCard(

diff --git a/ajax/rule.php b/ajax/rule.php
@@ -41,7 +41,7 @@
 switch ($_REQUEST['action']) {
     case "move_rule":
         if (is_subclass_of($_POST['collection_classname'], RuleCollection::getType())) {
-            $rule_collection = new $_POST['collection_classname']();
+            $rule_collection = getItemForItemtype($_POST['collection_classname']);
             $rule_collection->moveRule((int) $_POST['rule_id'], (int) $_POST['ref_id'], $_POST['sort_action']);
         }
         break;

diff --git a/ajax/updateTranslationFields.php b/ajax/updateTranslationFields.php
@@ -44,7 +44,7 @@
 
 Session::checkRight("dropdown", UPDATE);
 if (isset($_POST['itemtype']) && isset($_POST['language'])) {
-    $item = new $_POST['itemtype']();
+    $item = getItemForItemtype($_POST['itemtype']);
     $item->getFromDB($_POST['items_id']);
     DropdownTranslation::dropdownFields($item, $_POST['language']);
 }
diff --git a/ajax/updateTranslationValue.php b/ajax/updateTranslationValue.php
@@ -43,7 +43,7 @@
 $matching_field = null;
 
 if (isset($_POST['itemtype'], $_POST['field']) && is_a($_POST['itemtype'], CommonDropdown::class, true)) {
-    $itemtype = new $_POST['itemtype']();
+    $itemtype = getItemForItemtype($_POST['itemtype']);
     $matching_field = $itemtype->getAdditionalField($_POST['field']);
 }
 

diff --git a/composer.json b/composer.json
@@ -145,15 +145,17 @@
             "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-invalid-header-ignore.patch || true",
             "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-address-no-length-check.patch || true",
             "patch -f -p1 -d vendor/atoum/atoum/ < tools/patches/atoum-4.1.0...4.2.0.patch || true",
-            "patch -f -p1 -d vendor/michelf/php-markdown/ < tools/patches/php-markdown-php8-compat.patch || true"
+            "patch -f -p1 -d vendor/michelf/php-markdown/ < tools/patches/php-markdown-php8-compat.patch || true",
+            "patch -f -p1 -d vendor/guzzlehttp/guzzle/ < tools/patches/guzzle-http-client-restrict-http-methods.patch || true"
         ],
         "post-update-cmd": [
             "@php -r \"file_put_contents('.composer.hash', sha1_file('composer.lock'));\"",
             "@php -f vendor/bin/build_hw_jsons",
             "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-invalid-header-ignore.patch || true",
             "patch -f -p1 -d vendor/laminas/laminas-mail/ < tools/patches/laminas-mail-address-no-length-check.patch || true",
             "patch -f -p1 -d vendor/atoum/atoum/ < tools/patches/atoum-4.1.0...4.2.0.patch || true",
-            "patch -f -p1 -d vendor/michelf/php-markdown/ < tools/patches/php-markdown-php8-compat.patch || true"
+            "patch -f -p1 -d vendor/michelf/php-markdown/ < tools/patches/php-markdown-php8-compat.patch || true",
+            "patch -f -p1 -d vendor/guzzlehttp/guzzle/ < tools/patches/guzzle-http-client-restrict-http-methods.patch || true"
         ],
         "build": [
             "bin/console dependencies install && bin/console locales:compile"

diff --git a/front/device.php b/front/device.php
@@ -41,5 +41,5 @@
     );
 }
 
-$dropdown = new $_GET['itemtype']();
+$dropdown = getItemForItemtype($_GET['itemtype']);
 include(GLPI_ROOT . "/front/dropdown.common.php");
diff --git a/front/devicemodel.form.php b/front/devicemodel.form.php
@@ -41,5 +41,5 @@
     );
 }
 
-$dropdown = new $_GET['itemtype']();
+$dropdown = getItemForItemtype($_GET['itemtype']);
 include(GLPI_ROOT . "/front/dropdown.common.form.php");
diff --git a/front/devicemodel.php b/front/devicemodel.php
@@ -41,5 +41,5 @@
     );
 }
 
-$dropdown = new $_GET['itemtype']();
+$dropdown = getItemForItemtype($_GET['itemtype']);
 include(GLPI_ROOT . "/front/dropdown.common.php");
diff --git a/front/devicetype.form.php b/front/devicetype.form.php
@@ -41,5 +41,5 @@
     );
 }
 
-$dropdown = new $_GET['itemtype']();
+$dropdown = getItemForItemtype($_GET['itemtype']);
 include(GLPI_ROOT . "/front/dropdown.common.form.php");
diff --git a/front/devicetype.php b/front/devicetype.php
@@ -41,5 +41,5 @@
     );
 }
 
-$dropdown = new $_GET['itemtype']();
+$dropdown = getItemForItemtype($_GET['itemtype']);
 include(GLPI_ROOT . "/front/dropdown.common.php");
diff --git a/front/item_device.php b/front/item_device.php
@@ -44,7 +44,7 @@
     );
 }
 
-$itemDevice = new $_GET['itemtype']();
+$itemDevice = getItemForItemtype($_GET['itemtype']);
 if (!$itemDevice->canView()) {
     Session::redirectIfNotLoggedIn();
     Html::displayRightError();

diff --git a/front/itilfollowup.form.php b/front/itilfollowup.form.php
@@ -50,7 +50,7 @@
 if (!isset($_POST['itemtype']) || !class_exists($_POST['itemtype'])) {
     Html::displayErrorAndDie('Lost');
 }
-$track = new $_POST['itemtype']();
+$track = getItemForItemtype($_POST['itemtype']);
 
 
 if (isset($_POST["add"])) {

diff --git a/front/itilsolution.form.php b/front/itilsolution.form.php
@@ -43,7 +43,7 @@
 Session::checkLoginUser();
 
 $solution = new ITILSolution();
-$track = new $_POST['itemtype']();
+$track = getItemForItemtype($_POST['itemtype']);
 $track->getFromDB($_POST['items_id']);
 
 $redirect = null;

diff --git a/src/RuleAction.php b/src/RuleAction.php
@@ -51,7 +51,7 @@ public function getForbiddenStandardMassiveAction()
         $forbidden[] = 'update';
 
         if (isset($_POST['rule_class_name']) && is_subclass_of(\Rule::class, $_POST['rule_class_name'])) {
-            $rule = new $_POST['rule_class_name']();
+            $rule = getItemForItemtype($_POST['rule_class_name']);
             if ($rule->maxActionsCount() == 1) {
                 $forbidden[] = 'clone';
             }

diff --git a/tests/web/APIRest.php b/tests/web/APIRest.php
@@ -105,17 +105,11 @@ protected function doHttpRequest($verb = "get", $relative_uri = "", $params = []
            // Guzzle lib will automatically push the correct Content-type
             unset($params['headers']['Content-Type']);
         }
-        $verb = strtolower($verb);
-        if (in_array($verb, ['get', 'post', 'delete', 'put', 'options', 'patch'])) {
-            try {
-                return $this->http_client->{$verb}(
-                    $this->base_uri . $relative_uri,
-                    $params
-                );
-            } catch (\Throwable $e) {
-                throw $e;
-            }
-        }
+        return $this->http_client->request(
+            $verb,
+            $this->base_uri . $relative_uri,
+            $params
+        );
     }
 
     protected function query(

diff --git a/tools/patches/guzzle-http-client-restrict-http-methods.patch b/tools/patches/guzzle-http-client-restrict-http-methods.patch
@@ -0,0 +1,33 @@
+diff --git a/src/Client.php b/src/Client.php
+index 58f1d891..7448bb49 100644
+--- a/src/Client.php
++++ b/src/Client.php
+@@ -70,28 +70,6 @@ class Client implements ClientInterface, \Psr\Http\Client\ClientInterface
+         $this->configureDefaults($config);
+     }
+
+-    /**
+-     * @param string $method
+-     * @param array  $args
+-     *
+-     * @return PromiseInterface|ResponseInterface
+-     *
+-     * @deprecated Client::__call will be removed in guzzlehttp/guzzle:8.0.
+-     */
+-    public function __call($method, $args)
+-    {
+-        if (\count($args) < 1) {
+-            throw new InvalidArgumentException('Magic request methods require a URI and optional options array');
+-        }
+-
+-        $uri = $args[0];
+-        $opts = $args[1] ?? [];
+-
+-        return \substr($method, -5) === 'Async'
+-            ? $this->requestAsync(\substr($method, 0, -5), $uri, $opts)
+-            : $this->request($method, $uri, $opts);
+-    }
+-
+     /**
+      * Asynchronously send an HTTP request.
+      *