Skip to content

Commit

Permalink
Prevent XSS on generated links
Browse files Browse the repository at this point in the history
  • Loading branch information
cedric-anne authored and trasher committed Nov 3, 2022
1 parent a3ad777 commit 01c2172
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 19 deletions.
4 changes: 2 additions & 2 deletions front/link.send.php
Expand Up @@ -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']];
Expand Down
13 changes: 7 additions & 6 deletions src/CommonDBTM.php
Expand Up @@ -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);
}


Expand Down
9 changes: 7 additions & 2 deletions src/Domain.php
Expand Up @@ -33,6 +33,8 @@
* ---------------------------------------------------------------------
*/

use Glpi\Toolbox\URL;

/// Class Domain
class Domain extends CommonDBTM
{
Expand Down Expand Up @@ -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)
Expand Down
30 changes: 21 additions & 9 deletions src/Link.php
Expand Up @@ -33,6 +33,8 @@
* ---------------------------------------------------------------------
*/

use Glpi\Toolbox\URL;

/** Link Class
**/
class Link extends CommonDBTM
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -549,6 +555,9 @@ public static function generateLinkContents($link, CommonDBTM $item)
}

if ($disp) {
if ($safe_url) {
$tmplink = URL::sanitizeURL($tmplink) ?: '#';
}
$links[$key] = $tmplink;
}
}
Expand All @@ -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];
}

Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand Down
186 changes: 186 additions & 0 deletions tests/functionnal/Link.php
@@ -0,0 +1,186 @@
<?php

/**
* ---------------------------------------------------------------------
*
* GLPI - Gestionnaire Libre de Parc Informatique
*
* http://glpi-project.org
*
* @copyright 2015-2022 Teclib' and contributors.
* @copyright 2003-2014 by the INDEPNET Development Team.
* @licence https://www.gnu.org/licenses/gpl-3.0.html
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* ---------------------------------------------------------------------
*/

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' => <<<TEXT
id: [ID]
name: [NAME]
serial: [SERIAL]/[OTHERSERIAL]
location: [LOCATION] ([LOCATIONID])
domain: [DOMAIN] ([NETWORK])
owner: [USER]/[GROUP]
TEXT,
'item' => $item,
'safe_url' => $safe_url,
'expected' => [
<<<TEXT
id: {$item->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);
}
}

0 comments on commit 01c2172

Please sign in to comment.