Skip to content

Commit

Permalink
Prevent XSS on Inventory
Browse files Browse the repository at this point in the history
  • Loading branch information
@cedric-anne @trasher
cedric-anne authored and trasher committed Nov 3, 2022
1 parent 32e5ad6 commit 0898caa
Show file tree
Hide file tree
Showing 18 changed files with 116 additions and 106 deletions.
9 changes: 4 additions & 5 deletions src/Inventory/Asset/Antivirus.php
View file
Expand Up @@ -37,7 +37,7 @@

use ComputerAntivirus;
use Glpi\Inventory\Conf;
use Toolbox;
use Glpi\Toolbox\Sanitizer;

class Antivirus extends InventoryAsset
{
Expand Down Expand Up @@ -109,8 +109,6 @@ protected function getExisting(): array

public function handle()
{
global $DB;

$db_antivirus = $this->getExisting();
$value = $this->data;
$computerAntivirus = new ComputerAntivirus();
Expand All @@ -126,7 +124,7 @@ public function handle()
$input = $this->handleInput($val, $computerAntivirus) + [
'id' => $keydb
];
$computerAntivirus->update(Toolbox::addslashes_deep($input));
$computerAntivirus->update(Sanitizer::sanitize($input));
unset($value[$k]);
unset($db_antivirus[$keydb]);
break;
Expand All @@ -146,7 +144,8 @@ public function handle()
foreach ($value as $val) {
$val->computers_id = $this->item->fields['id'];
$val->is_dynamic = 1;
$computerAntivirus->add(Toolbox::addslashes_deep($this->handleInput($val, $computerAntivirus)));
$input = $this->handleInput($val, $computerAntivirus);
$computerAntivirus->add(Sanitizer::sanitize($input));
}
}
}
Expand Down
15 changes: 10 additions & 5 deletions src/Inventory/Asset/Camera.php
View file
Expand Up @@ -35,8 +35,8 @@

namespace Glpi\Inventory\Asset;

use CommonDBTM;
use Glpi\Inventory\Conf;
use Glpi\Toolbox\Sanitizer;
use Item_Devices;

class Camera extends Device
Expand Down Expand Up @@ -95,10 +95,12 @@ private function handleResolution($itemdevice, $val, $is_video = false)
continue;
}

$rsl = Sanitizer::sanitize($rsl);

$resolution = new \ImageResolution();
if (!$resolution->getFromDBByCrit(['name' => addslashes($rsl)])) {
if (!$resolution->getFromDBByCrit(['name' => $rsl])) {
$resolution->add([
'name' => addslashes($rsl),
'name' => $rsl,
'is_video' => $is_video,
'is_dynamic' => 1
]);
Expand Down Expand Up @@ -128,9 +130,12 @@ private function handleFormats($itemdevice, $val)
if (empty($fmt)) {
continue;
}
if (!$format->getFromDBByCrit(['name' => addslashes($fmt)])) {

$fmt = Sanitizer::sanitize($fmt);

if (!$format->getFromDBByCrit(['name' => $fmt])) {
$format->add([
'name' => addslashes($fmt),
'name' => $fmt,
'is_dynamic' => 1
]);
}
Expand Down
14 changes: 6 additions & 8 deletions src/Inventory/Asset/Cartridge.php
View file
Expand Up @@ -36,8 +36,8 @@
namespace Glpi\Inventory\Asset;

use Glpi\Inventory\Conf;
use Glpi\Toolbox\Sanitizer;
use Printer_CartridgeInfo;
use Toolbox;

class Cartridge extends InventoryAsset
{
Expand Down Expand Up @@ -230,8 +230,6 @@ protected function getExisting(): array

public function handle()
{
global $DB;

$cartinfo = new Printer_CartridgeInfo();
$db_cartridges = $this->getExisting();

Expand All @@ -243,7 +241,7 @@ public function handle()
'value' => $val,
'id' => $keydb
];
$cartinfo->update(Toolbox::addslashes_deep($input), false);
$cartinfo->update(Sanitizer::sanitize($input), false);
unset($value->$k);
unset($db_cartridges[$keydb]);
break;
Expand All @@ -259,11 +257,11 @@ public function handle()

foreach ($value as $property => $val) {
$cartinfo->add(
[
Sanitizer::sanitize([
'printers_id' => $this->item->fields['id'],
'property' => addslashes($property),
'value' => addslashes($val)
],
'property' => $property,
'value' => $val
]),
[],
false
);
Expand Down
10 changes: 5 additions & 5 deletions src/Inventory/Asset/DatabaseInstance.php
View file
Expand Up @@ -37,9 +37,9 @@

use DatabaseInstance as GDatabaseInstance;
use Glpi\Inventory\Conf;
use Glpi\Toolbox\Sanitizer;
use RuleImportAssetCollection;
use RuleMatchedLog;
use Toolbox;

class DatabaseInstance extends InventoryAsset
{
Expand Down Expand Up @@ -137,15 +137,15 @@ public function handle()
'itemtype' => $this->item->getType(),
'items_id' => $this->item->fields['id']
];
$items_id = $instance->add(Toolbox::addslashes_deep($input));
$items_id = $instance->add(Sanitizer::sanitize($input));
} else {
$items_id = $data['found_inventories'][0];
$databases = $val->databases ?? [];

$instance->getFromDB($items_id);
$input = $this->handleInput($val, $instance);
$input += ['id' => $instance->fields['id']];
$instance->update(Toolbox::addslashes_deep($input));
$instance->update(Sanitizer::sanitize($input));

$existing_databases = $instance->getDatabases();
//update databases, relying on name
Expand All @@ -154,7 +154,7 @@ public function handle()
if ($existing_database['name'] == $database->name) {
$dbinput = (array)$database;
$dbinput += ['id' => $dbkey, 'is_deleted' => 0, 'is_dynamic' => 1];
$odatabase->update(Toolbox::addslashes_deep($dbinput));
$odatabase->update(Sanitizer::sanitize($dbinput));
unset(
$existing_databases[$dbkey],
$databases[$key]
Expand All @@ -179,7 +179,7 @@ public function handle()
'databaseinstances_id' => $instance->fields['id'],
'is_dynamic' => 1
];
$odatabase->add(Toolbox::addslashes_deep($dbinput));
$odatabase->add(Sanitizer::sanitize($dbinput));
}

$instances[$items_id] = $items_id;
Expand Down
19 changes: 10 additions & 9 deletions src/Inventory/Asset/Device.php
View file
Expand Up @@ -35,7 +35,7 @@

namespace Glpi\Inventory\Asset;

use CommonDBTM;
use Glpi\Toolbox\Sanitizer;
use Item_Devices;

abstract class Device extends InventoryAsset
Expand Down Expand Up @@ -99,7 +99,8 @@ public function handle()

//create device or get existing device ID
$raw_input = $this->handleInput($val, $device);
$device_id = $device->import(\Toolbox::addslashes_deep($raw_input) + ['with_history' => false]);
$device_input = Sanitizer::dbEscapeRecursive($raw_input); // `handleInput` may copy unescaped values
$device_id = $device->import($device_input + ['with_history' => false]);

$i_criteria = $itemdevice->getImportCriteria();
$fk_input = [
Expand Down Expand Up @@ -150,28 +151,28 @@ public function handle()

if ($equals === true) {
$itemdevice->getFromDB($existing_item['id']);
$itemdevice_data = \Toolbox::addslashes_deep([
$itemdevice_data = [
'id' => $existing_item['id'],
$fk => $device_id,
'itemtype' => $this->item->getType(),
'items_id' => $this->item->fields['id'],
'is_dynamic' => 1
] + $this->handleInput($val, $itemdevice));
$itemdevice->update($itemdevice_data, false);
] + $this->handleInput($val, $itemdevice);
$itemdevice->update(Sanitizer::sanitize($itemdevice_data), false);
unset($existing[$device_id][$key]);
break;
}
}

if (($equals ?? false) !== true) {
$itemdevice->getEmpty();
$itemdevice_data = \Toolbox::addslashes_deep([
$itemdevice_data = [
$fk => $device_id,
'itemtype' => $this->item->getType(),
'items_id' => $this->item->fields['id'],
'is_dynamic' => 1
] + $this->handleInput($val, $itemdevice));
$itemdevice->add($itemdevice_data, [], false);
] + $this->handleInput($val, $itemdevice);
$itemdevice->add(Sanitizer::sanitize($itemdevice_data), [], false);
$this->itemdeviceAdded($itemdevice, $val);
}

Expand All @@ -181,7 +182,7 @@ public function handle()
}

//remove remaining devices instances
foreach ($existing as $deviceid => $data) {
foreach ($existing as $data) {
foreach ($data as $itemdevice_data) {
if ($itemdevice_data['is_dynamic'] == 1) {
$DB->delete(
Expand Down
8 changes: 4 additions & 4 deletions src/Inventory/Asset/InventoryAsset.php
View file
Expand Up @@ -232,24 +232,24 @@ public function handleLinks()
if (!isset($this->known_links[$known_key])) {
$entities_id = $this->entities_id;
if ($key == "locations_id") {
$this->known_links[$known_key] = Dropdown::importExternal('Location', addslashes($value->$key), $entities_id);
$this->known_links[$known_key] = Dropdown::importExternal('Location', $value->$key, $entities_id);
} else if (preg_match('/^.+models_id/', $key)) {
// models that need manufacturer relation for dictionary import
// see CommonDCModelDropdown::$additional_fields_for_dictionnary
$this->known_links[$known_key] = Dropdown::importExternal(
getItemtypeForForeignKeyField($key),
addslashes($value->$key),
$value->$key,
$entities_id,
['manufacturer' => $manufacturer_name]
);
} else if (isset($foreignkey_itemtype[$key])) {
$this->known_links[$known_key] = Dropdown::importExternal($foreignkey_itemtype[$key], addslashes($value->$key), $entities_id);
$this->known_links[$known_key] = Dropdown::importExternal($foreignkey_itemtype[$key], $value->$key, $entities_id);
} else if ($key !== 'entities_id' && $key !== 'states_id' && isForeignKeyField($key) && is_a($itemtype = getItemtypeForForeignKeyField($key), CommonDropdown::class, true)) {
$foreignkey_itemtype[$key] = $itemtype;

$this->known_links[$known_key] = Dropdown::importExternal(
$foreignkey_itemtype[$key],
addslashes($value->$key),
$value->$key,
$entities_id
);

Expand Down
28 changes: 12 additions & 16 deletions src/Inventory/Asset/InventoryNetworkPort.php
View file
Expand Up @@ -38,14 +38,14 @@

use DBmysqlIterator;
use Glpi\Inventory\Conf;
use Glpi\Toolbox\Sanitizer;
use IPAddress;
use IPNetwork;
use Item_DeviceNetworkCard;
use NetworkName;
use NetworkPort;
use NetworkPortAggregate;
use QueryParam;
use Toolbox;
use Unmanaged;

trait InventoryNetworkPort
Expand Down Expand Up @@ -169,10 +169,10 @@ private function cleanUnmanageds()
'itemtype' => $this->itemtype,
'items_id' => $this->items_id,
'is_dynamic' => 1,
'name' => addslashes($port->name)
'name' => $port->name,
];

$networkport->update($input);
$networkport->update(Sanitizer::sanitize($input));
$unmanaged->delete(['id' => $unmanageds_id], true);
}
}
Expand Down Expand Up @@ -238,7 +238,7 @@ private function handleIpNetworks()
'gateway' => $port->gateway,
'entities_id' => $this->entities_id
];
$ipnetwork->add(Toolbox::addslashes_deep($input));
$ipnetwork->add(Sanitizer::sanitize($input));
}
}
}
Expand All @@ -260,7 +260,6 @@ private function addNetworkPort(\stdClass $port)
unset($input[$key]);
}
}
$input = Toolbox::addslashes_deep($input);
$input = array_merge(
$input,
[
Expand All @@ -275,7 +274,7 @@ private function addNetworkPort(\stdClass $port)
$input['trunk'] = 0;
}

$netports_id = $networkport->add($input);
$netports_id = $networkport->add(Sanitizer::sanitize($input));
return $netports_id;
}

Expand All @@ -302,7 +301,7 @@ protected function addNetworkName($items_id, $name = null)
$input['name'] = $name;
}

$netname_id = $networkname->add($input);
$netname_id = $networkname->add(Sanitizer::sanitize($input));
return $netname_id;
}

Expand All @@ -321,10 +320,10 @@ private function addIPAddresses(array $ips, $items_id)
$input = [
'items_id' => $items_id,
'itemtype' => 'NetworkName',
'name' => addslashes($ip),
'name' => $ip,
'is_dynamic' => 1
];
$ipaddress->add($input);
$ipaddress->add(Sanitizer::sanitize($input));
}
}

Expand Down Expand Up @@ -355,9 +354,6 @@ private function handleUpdates()
if (is_null($row['mac'])) {
$row['mac'] = '';
}
if (preg_match("/[^a-zA-Z0-9 \-_\(\)]+/", $row['name'])) {
$row['name'] = Toolbox::addslashes_deep($row['name']);
}
foreach (['name', 'mac'] as $field) {
if ($row[$field] !== null) {
$row[$field] = strtolower($row[$field]);
Expand Down Expand Up @@ -398,10 +394,10 @@ private function handleUpdates()

if (property_exists($data, 'logical_number') && $data->logical_number != $db_lnumber) {
$networkport->update(
[
Sanitizer::sanitize([
'id' => $keydb,
'logical_number' => $data->logical_number
]
])
);
}

Expand Down Expand Up @@ -588,9 +584,9 @@ private function handleInstantiation($type, $data, $ports_id, $load)

//store instance
if ($instance->isNewItem()) {
$instance->add(Toolbox::addslashes_deep($input));
$instance->add(Sanitizer::sanitize($input));
} else {
$instance->update(Toolbox::addslashes_deep($input));
$instance->update(Sanitizer::sanitize($input));
}
}

Expand Down

0 comments on commit 0898caa

Please sign in to comment.