diff --git a/.gitignore b/.gitignore index 283db0afde..03a63885f2 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,5 @@ dist/ vendor/ .gh_token *.min.* +var/ +.phpunit.result.cache diff --git a/CHANGELOG.md b/CHANGELOG.md index 133b1dd8c4..7ed01bc373 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fix warning : `creation of dynamic property $table is deprecated` +- Fix missing rights checks +- Fix unescaped output in order/reception forms ## [2.11.5] - 2026-03-24 diff --git a/ajax/linkactions.php b/ajax/linkactions.php index e282c04756..41c1a5b90e 100644 --- a/ajax/linkactions.php +++ b/ajax/linkactions.php @@ -29,6 +29,10 @@ */ include("../../../inc/includes.php"); + +/** @var array $CFG_GLPI */ +global $CFG_GLPI; + header("Content-Type: text/html; charset=UTF-8"); Html::header_nocache(); Session::checkLoginUser(); @@ -175,6 +179,15 @@ break; case "check_unicity": + Session::checkRight('plugin_order_order', READ); + + if ( + !in_array($_POST['itemtype'] ?? '', $CFG_GLPI['asset_types'], true) + || !in_array($_POST['field'] ?? '', ['name', 'serial'], true) + ) { + throw new RuntimeException("Invalid item type or field"); + } + $itemtype = new $_POST['itemtype'](); if (count($itemtype->find([$_POST['field'] => $_POST['field_value'], ])) > 0) { echo "false"; diff --git a/ajax/massreception.php b/ajax/massreception.php index 30bc83eeaf..db52d258b8 100644 --- a/ajax/massreception.php +++ b/ajax/massreception.php @@ -35,6 +35,8 @@ Html::header_nocache(); Session::checkLoginUser(); +Session::checkRight('plugin_order_order', READ); +(new PluginOrderOrder())->check($_POST['plugin_order_orders_id'], READ); $PluginOrderReception = new PluginOrderReception(); diff --git a/front/documentcategory.form.php b/front/documentcategory.form.php index f4a5dcab87..ef54afc26a 100644 --- a/front/documentcategory.form.php +++ b/front/documentcategory.form.php @@ -30,6 +30,7 @@ include('../../../inc/includes.php'); Session::checkLoginUser(); +Session::checkRight("config", UPDATE); $documentCategory = new PluginOrderDocumentCategory(); diff --git a/front/order.form.php b/front/order.form.php index 4c44565942..02c9f5caef 100644 --- a/front/order.form.php +++ b/front/order.form.php @@ -88,6 +88,7 @@ //Status update & order workflow } else if (isset($_POST["validate"])) { /* validate order */ + $pluginOrderOrder->check($_POST["id"], UPDATE); if (PluginOrderOrder::canView() && (PluginOrderOrder::canValidate() || !$config->useValidation())) { $pluginOrderOrder->updateOrderStatus($_POST["id"], $config->getApprovedState(), $_POST["comment"]); PluginOrderReception::updateDelivryStatus($_POST["id"]); @@ -95,6 +96,7 @@ } Html::back(); } else if (isset($_POST["waiting_for_approval"])) { + $pluginOrderOrder->check($_POST["id"], UPDATE); if (PluginOrderOrder::canCreate()) { $pluginOrderOrder->updateOrderStatus( $_POST["id"], @@ -105,6 +107,7 @@ } Html::back(); } else if (isset($_POST["cancel_waiting_for_approval"])) { + $pluginOrderOrder->check($_POST["id"], UPDATE); if (PluginOrderOrder::canView() && PluginOrderOrder::canCancel()) { $pluginOrderOrder->updateOrderStatus( $_POST["id"], @@ -116,6 +119,7 @@ Html::back(); } else if (isset($_POST["cancel_order"])) { + $pluginOrderOrder->check($_POST["id"], UPDATE); if (PluginOrderOrder::canView() && PluginOrderOrder::canCancel()) { $pluginOrderOrder->updateOrderStatus( $_POST["id"], @@ -128,6 +132,7 @@ Html::back(); } else if (isset($_POST["undovalidation"])) { + $pluginOrderOrder->check($_POST["id"], UPDATE); if (PluginOrderOrder::canView() && PluginOrderOrder::canUndo()) { $pluginOrderOrder->updateOrderStatus( $_POST["id"], @@ -140,6 +145,7 @@ Html::back(); } else if (isset($_POST["add_item"])) { //Details management + $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); if ($_POST["discount"] < 0 || $_POST["discount"] > 100) { Session::addMessageAfterRedirect(__("The discount pourcentage must be between 0 and 100", "order"), false, ERROR); } else { @@ -170,6 +176,7 @@ } Html::back(); } else if (isset($_POST["delete_item"])) { + $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); if ( isset($_POST["plugin_order_orders_id"]) && ($_POST["plugin_order_orders_id"] > 0) @@ -227,6 +234,11 @@ $new_value, $_POST["plugin_order_orders_id"] ); + + if (!$pluginOrderOrder_Item->belongsToOrder((int) $_POST["plugin_order_orders_id"])) { + continue; + } + $pluginOrderOrder_Item->delete(['id' => $ID]); } } @@ -238,6 +250,7 @@ Html::back(); } else if (isset($_POST["add_itemfree"])) { //Details management + $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); if ($_POST["discount"] < 0 || $_POST["discount"] > 100) { Session::addMessageAfterRedirect(__("The discount pourcentage must be between 0 and 100", "order"), false, ERROR); } else { @@ -313,6 +326,7 @@ } Html::back(); } else if (isset($_POST["delete_itemfree"])) { + $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); if ( isset($_POST["plugin_order_orders_id"]) && ($_POST["plugin_order_orders_id"] > 0) @@ -364,12 +378,18 @@ $new_value, $_POST["plugin_order_orders_id"] ); + + if (!$pluginOrderOrder_Item->belongsToOrder((int) $_POST["plugin_order_orders_id"])) { + continue; + } + $pluginOrderOrder_Item->delete(['id' => $ID]); } } } } } else if (isset($_POST["update_item"])) { + $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); if (isset($_POST['quantity'])) { $pluginOrderOrder_Item->updateQuantity($_POST); } @@ -430,6 +450,7 @@ Html::back(); } else if (isset($_POST["update_detail_item"])) { + $pluginOrderOrder->check($_POST["plugin_order_orders_id"], UPDATE); if (isset($_POST['detail_price_taxfree'])) { foreach ($_POST['detail_price_taxfree'] as $item_id => $price) { $pluginOrderOrder_Item->updatePrice_taxfree([ diff --git a/front/order_item.form.php b/front/order_item.form.php index b3670ced32..2253ca55aa 100644 --- a/front/order_item.form.php +++ b/front/order_item.form.php @@ -33,6 +33,8 @@ $item = new PluginOrderOrder_Item(); if (isset($_POST['update'])) { + $item->getFromDB($_POST['id']); + (new PluginOrderOrder())->check($item->fields['plugin_order_orders_id'], UPDATE); $item->update($_POST); $item->updatePrices($_POST['id']); Html::redirect($_SERVER['HTTP_REFERER']); diff --git a/front/reception.form.php b/front/reception.form.php index 783da882f1..be337297c9 100644 --- a/front/reception.form.php +++ b/front/reception.form.php @@ -45,8 +45,9 @@ $order_item = new PluginOrderOrder_Item(); if (isset($_POST["update"])) { + $order_item->getFromDB($_POST["id"]); + (new PluginOrderOrder())->check($order_item->fields["plugin_order_orders_id"], UPDATE); if (PluginOrderReception::canCreate()) { - $order_item->getFromDB($_POST["id"]); if ($order_item->fields["itemtype"] == 'SoftwareLicense') { $result = $order_item->queryRef( $order_item->fields["plugin_order_orders_id"], @@ -82,6 +83,8 @@ ]); Html::redirect($_SERVER['HTTP_REFERER']); } else if (isset($_POST["delete"])) { + $order_item->getFromDB($_POST["id"]); + (new PluginOrderOrder())->check($order_item->fields["plugin_order_orders_id"], UPDATE); $reception->deleteDelivery($_POST["id"]); $reception->updateReceptionStatus([ 'items' => [ @@ -93,6 +96,7 @@ Html::redirect(Toolbox::getItemTypeFormURL('PluginOrderOrder') . "?id=" . $_POST["plugin_order_orders_id"]); } else if (isset($_POST["bulk_reception"])) { //Several new items are delivered + (new PluginOrderOrder())->check($_POST["plugin_order_orders_id"], UPDATE); $reception->updateBulkReceptionStatus($_POST); Html::redirect($_SERVER["HTTP_REFERER"]); } else { diff --git a/hook.php b/hook.php index 17b03dc212..6575604683 100644 --- a/hook.php +++ b/hook.php @@ -132,7 +132,7 @@ function plugin_order_uninstall() 'PluginOrderOrderType', 'PluginOrderOther', 'PluginOrderOtherType', 'PluginOrderPreference', 'PluginOrderProfile', 'PluginOrderReference_Supplier', 'PluginOrderSurveySupplier', 'PluginOrderDocumentCategory', - 'PluginOrderAccountSection', 'PluginOrderAnalyticNature' + 'PluginOrderAccountSection', 'PluginOrderAnalyticNature', 'PluginOrderReferenceFree' ]; foreach ($classes as $class) { call_user_func([$class, 'uninstall']); diff --git a/inc/order.class.php b/inc/order.class.php index 412d4cd7cc..836926cb45 100644 --- a/inc/order.class.php +++ b/inc/order.class.php @@ -1818,6 +1818,15 @@ public function generateOrder($params) throw new \RuntimeException("Invalid template name"); } + // Avoid access to another directory or to files that does not match allowed extension + $extensionPattern = '/\.(' . implode('|', array_map(fn($ext) => preg_quote((string) $ext, '/'), PLUGIN_ORDER_SIGNATURE_EXTENSION)) . ')$/'; + if ( + !empty($signature) + && (preg_match('/[\\\\\/]/', $signature) !== 0 || preg_match($extensionPattern, $signature) === 0) + ) { + throw new RuntimeException("Invalid signature file name"); + } + $template_path = PLUGIN_ORDER_TEMPLATE_DIR . $template; // Ensure the file exists and is readable diff --git a/inc/order_item.class.php b/inc/order_item.class.php index f9d485c82a..d145b768dc 100644 --- a/inc/order_item.class.php +++ b/inc/order_item.class.php @@ -312,6 +312,16 @@ public function getPricesATI($priceHT, $taxes) return (!$priceHT ? 0 : $priceHT + (($priceHT * $taxes) / 100)); } + /** + * Check that this item belongs to the given order, to prevent cross-order IDOR on deletion. + * + * @param int $orders_id Order ID expected to own this item + */ + public function belongsToOrder(int $orders_id): bool + { + return (int) $this->fields['plugin_order_orders_id'] === $orders_id; + } + /** * Calculate the total ecotax from all ordered items with their quantities * diff --git a/inc/profile.class.php b/inc/profile.class.php index 421440d694..2af6f90c73 100644 --- a/inc/profile.class.php +++ b/inc/profile.class.php @@ -267,10 +267,11 @@ public static function migrateOneProfile($profiles_id) } break; } - $query = "UPDATE `glpi_profilerights` - SET `rights`='" . $right . "' - WHERE `name`='$new' AND `profiles_id`='$profiles_id'"; - $DB->query($query); + $DB->update( + 'glpi_profilerights', + ['rights' => $right], + ['name' => $new, 'profiles_id' => $profiles_id], + ); } } } diff --git a/inc/referencefree.class.php b/inc/referencefree.class.php index 7f40a2d902..9cbfd19ef3 100644 --- a/inc/referencefree.class.php +++ b/inc/referencefree.class.php @@ -107,20 +107,21 @@ public static function install(Migration $migration) public static function uninstall() { - /** @var \DBmysql $DB */ + /** @var DBmysql $DB */ global $DB; - $table = getTableForItemType(__CLASS__); + $table = getTableForItemType(self::class); foreach ( ["glpi_displaypreferences", "glpi_documents_items", "glpi_savedsearches", - "glpi_logs" + "glpi_logs", ] as $t ) { - $query = "DELETE FROM `$t` WHERE `itemtype`='" . __CLASS__ . "'"; - $DB->query($query); + $itemtype = getItemTypeForTable($t); + $item = getItemForItemtype($itemtype); + $item->deleteByCriteria(['itemtype' => self::class]); } - $DB->query("DROP TABLE IF EXISTS `$table`") or die($DB->error()); + $DB->doQuery(sprintf('DROP TABLE IF EXISTS `%s`', $table)); } /** diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000000..3023d8c5ff --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,7 @@ + + + + tests + + + diff --git a/tests/Units/DocumentCategoryTest.php b/tests/Units/DocumentCategoryTest.php new file mode 100644 index 0000000000..a4b2f81ee5 --- /dev/null +++ b/tests/Units/DocumentCategoryTest.php @@ -0,0 +1,62 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2009-2026 by Order plugin team. + * @license GPLv3 https://www.gnu.org/licenses/gpl-3.0.html + * @link https://github.com/pluginsGLPI/order + * ------------------------------------------------------------------------- + */ + +declare(strict_types=1); + +namespace GlpiPlugin\Order\Tests\Units; + +use DbTestCase; +use DocumentCategory; +use Glpi\Toolbox\Sanitizer; +use PluginOrderDocumentCategory; + +final class DocumentCategoryTest extends DbTestCase +{ + private const XSS_PAYLOAD = ''; + + public function testShowForDocumentCategoryEscapesPrefix(): void + { + $document_category = $this->createItem(DocumentCategory::class, [ + 'name' => $this->getUniqueString(), + ]); + + $this->createItem(PluginOrderDocumentCategory::class, [ + 'documentcategories_id' => $document_category->getID(), + 'documentcategories_prefix' => self::XSS_PAYLOAD, + ]); + + ob_start(); + PluginOrderDocumentCategory::showForDocumentCategory($document_category); + $output = ob_get_clean(); + + $this->assertStringNotContainsString(self::XSS_PAYLOAD, $output); + $this->assertStringContainsString(Sanitizer::sanitize(self::XSS_PAYLOAD), $output); + } +} diff --git a/tests/Units/OrderItemTest.php b/tests/Units/OrderItemTest.php new file mode 100644 index 0000000000..77ecac636e --- /dev/null +++ b/tests/Units/OrderItemTest.php @@ -0,0 +1,55 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2009-2026 by Order plugin team. + * @license GPLv3 https://www.gnu.org/licenses/gpl-3.0.html + * @link https://github.com/pluginsGLPI/order + * ------------------------------------------------------------------------- + */ + +declare(strict_types=1); + +namespace GlpiPlugin\Order\Tests\Units; + +use DbTestCase; +use PluginOrderOrder_Item; + +final class OrderItemTest extends DbTestCase +{ + public function testBelongsToOrderReturnsTrueForOwningOrder(): void + { + $item = new PluginOrderOrder_Item(); + $item->fields['plugin_order_orders_id'] = 5; + + $this->assertTrue($item->belongsToOrder(5)); + } + + public function testBelongsToOrderReturnsFalseForForeignOrder(): void + { + $item = new PluginOrderOrder_Item(); + $item->fields['plugin_order_orders_id'] = 5; + + $this->assertFalse($item->belongsToOrder(42)); + } +} diff --git a/tests/Units/OrderTest.php b/tests/Units/OrderTest.php new file mode 100644 index 0000000000..a7489e2038 --- /dev/null +++ b/tests/Units/OrderTest.php @@ -0,0 +1,107 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2009-2026 by Order plugin team. + * @license GPLv3 https://www.gnu.org/licenses/gpl-3.0.html + * @link https://github.com/pluginsGLPI/order + * ------------------------------------------------------------------------- + */ + +declare(strict_types=1); + +namespace GlpiPlugin\Order\Tests\Units; + +use DbTestCase; +use PluginOrderOrder; +use RuntimeException; + +final class OrderTest extends DbTestCase +{ + public static function invalidTemplateNameProvider(): iterable + { + yield 'path traversal' => ['../../../etc/passwd.odt']; + yield 'embedded path separator' => ['sub/template.odt']; + yield 'disallowed extension' => ['template.docx']; + } + + /** @dataProvider invalidTemplateNameProvider */ + public function testGenerateOrderRejectsInvalidTemplateName(string $template): void + { + $order = new PluginOrderOrder(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Invalid template name'); + + $order->generateOrder([ + 'id' => 0, + 'template' => $template, + 'sign' => '', + ]); + } + + public static function invalidSignatureNameProvider(): iterable + { + yield 'path traversal via slash' => ['../../../etc/passwd.png']; + yield 'path traversal via backslash' => ['..\\..\\signature.png']; + yield 'disallowed extension' => ['signature.php']; + } + + /** @dataProvider invalidSignatureNameProvider */ + public function testGenerateOrderRejectsInvalidSignatureName(string $signature): void + { + $order = new PluginOrderOrder(); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Invalid signature file name'); + + $order->generateOrder([ + 'id' => 0, + 'template' => 'template.odt', + 'sign' => $signature, + ]); + } + + public static function acceptedSignatureProvider(): iterable + { + yield 'no signature' => ['']; + yield 'valid png signature' => ['signature.png']; + } + + /** @dataProvider acceptedSignatureProvider */ + public function testGenerateOrderAcceptsValidSignatureAndReachesFileCheck(string $signature): void + { + $order = new PluginOrderOrder(); + + // The template file does not exist in the test environment: reaching this + // exception proves the name/signature validation passed successfully. + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Template file not found or not readable'); + + $order->generateOrder([ + 'id' => 0, + 'template' => 'template.odt', + 'sign' => $signature, + ]); + } +} diff --git a/tests/Units/ReferenceTest.php b/tests/Units/ReferenceTest.php new file mode 100644 index 0000000000..85849503bc --- /dev/null +++ b/tests/Units/ReferenceTest.php @@ -0,0 +1,79 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2009-2026 by Order plugin team. + * @license GPLv3 https://www.gnu.org/licenses/gpl-3.0.html + * @link https://github.com/pluginsGLPI/order + * ------------------------------------------------------------------------- + */ + +declare(strict_types=1); + +namespace GlpiPlugin\Order\Tests\Units; + +use DbTestCase; +use Glpi\Toolbox\Sanitizer; +use PluginOrderReference; +use ProfileRight; + +final class ReferenceTest extends DbTestCase +{ + private const XSS_PAYLOAD = ''; + + private function removeRightFromProfile(string $profileName, string $rightName, int $right): void + { + $profiles_id = getItemByTypeName('Profile', $profileName, true); + $current = ProfileRight::getProfileRights($profiles_id)[$rightName] ?? 0; + + ProfileRight::updateProfileRights($profiles_id, [ + $rightName => $current & ~$right, + ]); + } + + public function testGetReceptionReferenceLinkEscapesNameWhenLinked(): void + { + $reference = new PluginOrderReference(); + $link = $reference->getReceptionReferenceLink([ + 'id' => 1, + 'name' => Sanitizer::sanitize(self::XSS_PAYLOAD), + ]); + + $this->assertStringNotContainsString(self::XSS_PAYLOAD, $link); + $this->assertStringContainsString(Sanitizer::sanitize(self::XSS_PAYLOAD), $link); + } + + public function testGetReceptionReferenceLinkEscapesNameWithoutViewRight(): void + { + $this->removeRightFromProfile('Super-Admin', PluginOrderReference::$rightname, READ); + + $reference = new PluginOrderReference(); + $name = $reference->getReceptionReferenceLink([ + 'id' => 1, + 'name' => Sanitizer::sanitize(self::XSS_PAYLOAD), + ]); + + $this->assertStringNotContainsString(self::XSS_PAYLOAD, $name); + $this->assertSame(Sanitizer::sanitize(self::XSS_PAYLOAD), $name); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000000..c5db066da8 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,49 @@ +. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2009-2026 by Order plugin team. + * @license GPLv3 https://www.gnu.org/licenses/gpl-3.0.html + * @link https://github.com/pluginsGLPI/order + * ------------------------------------------------------------------------- + */ + +/** @var array $CFG_GLPI */ +/** @var array $PLUGIN_HOOKS */ +global $CFG_GLPI, $PLUGIN_HOOKS; + +define('GLPI_ROOT', __DIR__ . '/../../../'); +define('GLPI_LOG_DIR', __DIR__ . '/files/_logs'); + +define('TU_USER', 'glpi'); +define('TU_PASS', 'glpi'); +define('GLPI_LOG_LVL', 'DEBUG'); + +require GLPI_ROOT . '/inc/includes.php'; + +Plugin::load('order', true); + +include_once GLPI_ROOT . '/phpunit/GLPITestCase.php'; +include_once GLPI_ROOT . '/phpunit/DbTestCase.php'; +require_once __DIR__ . '/../vendor/autoload.php'; +require_once __DIR__ . '/../setup.php';