diff --git a/front/link.send.php b/front/link.send.php index e328f1d0473..710d0138442 100644 --- a/front/link.send.php +++ b/front/link.send.php @@ -53,8 +53,8 @@ if ($item = getItemForItemtype($_GET["itemtype"])) { if ($item->getFromDB($_GET["id"])) { - $content_filename = Link::generateLinkContents($link, $item); - $content_data = Link::generateLinkContents($file, $item); + $content_filename = Link::generateLinkContents($link, $item, false); + $content_data = Link::generateLinkContents($file, $item, false); if (isset($_GET['rank']) && isset($content_filename[$_GET['rank']])) { $filename = $content_filename[$_GET['rank']]; diff --git a/src/CommonDBTM.php b/src/CommonDBTM.php index c753a9634aa..2b142701f7d 100644 --- a/src/CommonDBTM.php +++ b/src/CommonDBTM.php @@ -5345,18 +5345,19 @@ public function getRights($interface = 'central') } /** - * Generate link + * Generate link(s). * * @since 9.1 * - * @param string $link original string content - * @param CommonDBTM $item item used to make replacements + * @param string $link original string content + * @param CommonDBTM $item item used to make replacements + * @param bool $safe_url indicates whether URL should be sanitized or not * * @return array of link contents (may have several when item have several IP / MAC cases) - **/ - public static function generateLinkContents($link, CommonDBTM $item) + */ + public static function generateLinkContents($link, CommonDBTM $item, bool $safe_url = true) { - return Link::generateLinkContents($link, $item); + return Link::generateLinkContents($link, $item, $safe_url); } diff --git a/src/Domain.php b/src/Domain.php index 6d861048f9c..6a692904a7a 100644 --- a/src/Domain.php +++ b/src/Domain.php @@ -33,6 +33,8 @@ * --------------------------------------------------------------------- */ +use Glpi\Toolbox\URL; + /// Class Domain class Domain extends CommonDBTM { @@ -725,14 +727,17 @@ public static function getTypes($all = false) return $types; } - public static function generateLinkContents($link, CommonDBTM $item) + public static function generateLinkContents($link, CommonDBTM $item, bool $safe_url = true) { if (strstr($link, "[DOMAIN]")) { $link = str_replace("[DOMAIN]", $item->getName(), $link); + if ($safe_url) { + $link = URL::sanitizeURL($link) ?: '#'; + } return [$link]; } - return parent::generateLinkContents($link, $item); + return parent::generateLinkContents($link, $item, $safe_url); } public static function getUsed(array $used, $domaintype) diff --git a/src/Link.php b/src/Link.php index 80cd5c155b3..b77067e9ce0 100644 --- a/src/Link.php +++ b/src/Link.php @@ -33,6 +33,8 @@ * --------------------------------------------------------------------- */ +use Glpi\Toolbox\URL; + /** Link Class **/ class Link extends CommonDBTM @@ -259,14 +261,15 @@ public function rawSearchOptions() /** - * Generate link + * Generate link(s). * - * @param $link string original string content - * @param $item CommonDBTM object: item used to make replacements + * @param string $link original string content + * @param CommonDBTM $item item used to make replacements + * @param bool $safe_url indicates whether URL should be sanitized or not * * @return array of link contents (may have several when item have several IP / MAC cases) - **/ - public static function generateLinkContents($link, CommonDBTM $item) + */ + public static function generateLinkContents($link, CommonDBTM $item, bool $safe_url = true) { global $DB, $CFG_GLPI; @@ -405,6 +408,9 @@ public static function generateLinkContents($link, CommonDBTM $item) $replace_MAC = strstr($link, "[MAC]"); if (!$replace_IP && !$replace_MAC) { + if ($safe_url) { + $link = URL::sanitizeURL($link) ?: '#'; + } return [$link]; } // Return several links id several IP / MAC @@ -549,6 +555,9 @@ public static function generateLinkContents($link, CommonDBTM $item) } if ($disp) { + if ($safe_url) { + $tmplink = URL::sanitizeURL($tmplink) ?: '#'; + } $links[$key] = $tmplink; } } @@ -557,6 +566,9 @@ public static function generateLinkContents($link, CommonDBTM $item) if (count($links)) { return $links; } + if ($safe_url) { + $link = URL::sanitizeURL($link) ?: '#'; + } return [$link]; } @@ -639,12 +651,12 @@ public static function getAllLinksFor($item, $params = []) $params['name'] = $params['link']; } - $names = $item->generateLinkContents($params['name'], $item); + $names = $item->generateLinkContents($params['name'], $item, false); $file = trim($params['data']); if (empty($file)) { // Generate links - $links = $item->generateLinkContents($params['link'], $item); + $links = $item->generateLinkContents($params['link'], $item, true); $i = 1; foreach ($links as $key => $val) { $name = (isset($names[$key]) ? $names[$key] : reset($names)); @@ -662,8 +674,8 @@ public static function getAllLinksFor($item, $params = []) } } else { // Generate files - $files = $item->generateLinkContents($params['link'], $item); - $links = $item->generateLinkContents($params['data'], $item); + $files = $item->generateLinkContents($params['link'], $item, false); + $links = $item->generateLinkContents($params['data'], $item, false); $i = 1; foreach ($links as $key => $val) { $name = (isset($names[$key]) ? $names[$key] : reset($names)); diff --git a/tests/functionnal/Link.php b/tests/functionnal/Link.php new file mode 100644 index 00000000000..033320bd9b2 --- /dev/null +++ b/tests/functionnal/Link.php @@ -0,0 +1,186 @@ +. + * + * --------------------------------------------------------------------- + */ + +namespace tests\units; + +use DbTestCase; + +class Link extends DbTestCase +{ + protected function linkContentProvider(): iterable + { + $this->login(); + + // Create network + $network = $this->createItem( + \Network::class, + [ + 'name' => 'LAN', + ] + ); + + // Create computer + $item = $this->createItem( + \Computer::class, + [ + 'name' => 'Test computer', + 'serial' => 'ABC0004E6', + 'otherserial' => 'X0000015', + 'uuid' => 'c938f085-4192-4473-a566-46734bbaf6ad', + 'entities_id' => $_SESSION['glpiactive_entity'], + 'groups_id' => getItemByTypeName(\Group::class, '_test_group_1', true), + 'locations_id' => getItemByTypeName(\Location::class, '_location01', true), + 'networks_id' => $network->getID(), + 'users_id' => getItemByTypeName(\User::class, 'glpi', true), + ] + ); + + // Attach domains + $domain1 = $this->createItem( + \Domain::class, + [ + 'name' => 'domain1.tld', + 'entities_id' => $_SESSION['glpiactive_entity'], + ] + ); + $this->createItem( + \Domain_Item::class, + [ + 'domains_id' => $domain1->getID(), + 'itemtype' => \Computer::class, + 'items_id' => $item->getID(), + ] + ); + $domain2 = $this->createItem( + \Domain::class, + [ + 'name' => 'domain2.tld', + 'entities_id' => $_SESSION['glpiactive_entity'], + ] + ); + $this->createItem( + \Domain_Item::class, + [ + 'domains_id' => $domain2->getID(), + 'itemtype' => \Computer::class, + 'items_id' => $item->getID(), + ] + ); + + // Empty link + yield [ + 'link' => '', + 'item' => $item, + 'safe_url' => false, + 'expected' => [''], + ]; + yield [ + 'link' => '', + 'item' => $item, + 'safe_url' => true, + 'expected' => ['#'], + ]; + + foreach ([true, false] as $safe_url) { + // Link that is actually a title (it is a normal usage!) + yield [ + 'link' => '[LOCATION] > [SERIAL] ([USER])', + 'item' => $item, + 'safe_url' => $safe_url, + 'expected' => ['_location01 > ABC0004E6 (glpi)'], + ]; + + // Link that is actually a long text (it is a normal usage!) + yield [ + 'link' => << $item, + 'safe_url' => $safe_url, + 'expected' => [ + <<getID()} +name: Test computer +serial: ABC0004E6/X0000015 +location: _location01 (1) +domain: domain1.tld (LAN) +owner: glpi/_test_group_1 +TEXT, + ], + ]; + + // Valid http link + yield [ + 'link' => 'https://[LOGIN]@[DOMAIN]/[FIELD:uuid]/', + 'item' => $item, + 'safe_url' => $safe_url, + 'expected' => ['https://_test_user@domain1.tld/c938f085-4192-4473-a566-46734bbaf6ad/'], + ]; + } + + // Javascript link + yield [ + 'link' => 'javascript:alert(1);" title="[NAME]"', + 'item' => $item, + 'safe_url' => false, + 'expected' => ['javascript:alert(1);" title="Test computer"'], + ]; + yield [ + 'link' => 'javascript:alert(1);" title="[NAME]"', + 'item' => $item, + 'safe_url' => true, + 'expected' => ['#'], + ]; + } + + /** + * @dataProvider linkContentProvider + */ + public function testGenerateLinkContents( + string $link, + \CommonDBTM $item, + bool $safe_url, + array $expected + ): void { + $this->newTestedInstance(); + $this->array($this->testedInstance->generateLinkContents($link, $item, $safe_url)) + ->isEqualTo($expected); + } +}